[Gluster-devel] Mixing style and other changes in a patch
Niels de Vos
ndevos at redhat.com
Thu Feb 16 14:45:28 UTC 2017
On Thu, Feb 16, 2017 at 08:41:23AM -0500, Jeff Darcy wrote:
> In the last few days, I've seen both of these kinds of review comments
> (not necessarily on my own patches or from the same reviewers).
> (a) "Please fix the style in the entire function where you changed one line."
> (b) "This style change should be in a separate patch."
> It's clearly not helpful to have patches delayed for both reasons.
> Which should prevail? I think our general practice has been more
> toward (b) and that's also my personal preference. In that case
> instances of (a) should not occur. Or maybe people feel it should be
> the other way around. Can we get a consensus here?
I do not like to have patches change the coding style alone. Only for
the lines at the beginning of the function I prefer to see (a) being
followed, but blocking a patch should not be the case. Still, if some of
the regular contributors do not follow the coding-style, I tend to get
annoyed with it and my give them a -1 in the hope they do repeat that in
the future. If it is really the only thing that bothers me, I tend to
send an updated patch for them (after a chat on IRC).
Doing coding-style fixes on lines that have no other modifications tend
to make 'git blame' more difficult to follow. I use that a lot when
trying to figure out when a problem has been introduced. So in general,
I really do not like (b).
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 801 bytes
Desc: not available
More information about the Gluster-devel