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
- Whitespace and other stylistic changes · Maarten Bosmans
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.