Hi, On Mon, Mar 5, 2012 at 3:34 AM, Tor Lillqvist <[email protected]> wrote: > Does anybody who knows the code feel like reviewing the patch for > real,
I could review, but then I'm not knowledgeable enough on the chart2 code to add any confidence. or should we just trust Markus that it fixes the problem, and > push it? (I think we should.) I'm not so sure, since even Markus says in his post he doesn't really understand the code that he's modified, and I'm not either. > Isn't there some odd indentation changes in the patch? The same pattern is used in the first part of the if statement, and his patch only modifies the second part. Is that a legitimate change, or should the first part be modified also? I don't know the chart2 code well enough to answer that question. Also, what would be the risk of this change; what functionalities of the chart be affected by this change? My honest recommendation is to keep this change in master only for the time being, and consider backporting when we feel more confident of this change. I've been burned in the past by the very changes I made in chart2 which I though were safe but ended up causing regressions.... Sorry about not giving my thumbs-up. Kohei _______________________________________________ LibreOffice mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/libreoffice
