Date: prev next · Thread: first prev next last
2016 Archives by date, by thread · List index


Hi devs,

On a recent gerrit patch submission of mine, Markus Mohrhard added the
following comment:

Please don't mix style changes with functional changes. So please split the patch into 3: the 
actual change mentioned in the commit msg, the translations and the whitespace change.

Actually leave the whitespace changes out. The whitespace change just introduces noise and 
conflicts with the code style used by other developers. In general it is a bad idea to try to 
enforce a different coding style than the one used in a file already.

I understand that it is not worth the reviewing time, merge conflicts
and git blame noise to change the code style used in a file to another
one.
The problem here is that the file I was working on had no consistent style.
For example I saw both `if (foo)` and `if ( bar )`, sometimes even in
the same function.

So my question is: how far away from a functional change is it
desired/allowed to make stylistic changes for consistency? Is that the
whole file, class, function, scope block, statement or line?
I guess it is OK to change whitespace and other stuff on a line where
you already are making a functional change. Perhaps it's even required
to pass review?

Maarten

Context


Privacy Policy | Impressum (Legal Info) | Copyright information: Unless otherwise specified, all text and images on this website are licensed under the Creative Commons Attribution-Share Alike 3.0 License. This does not include the source code of LibreOffice, which is licensed under the Mozilla Public License (MPLv2). "LibreOffice" and "The Document Foundation" are registered trademarks of their corresponding registered owners or are in actual use as trademarks in one or more countries. Their respective logos and icons are also subject to international copyright laws. Use thereof is explained in our trademark policy.