Whitespace and other stylistic changes

Maarten Bosmans mkbosmans at gmail.com
Sun Sep 25 18:14:37 UTC 2016


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


More information about the LibreOffice mailing list