My preference would be that formatting fixes to existing code should be a separate commit.
On February 16, 2017 6:45:28 AM PST, Niels de Vos <ndevos@xxxxxxxxxx> wrote:
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).
Niels
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
_______________________________________________ Gluster-devel mailing list Gluster-devel@xxxxxxxxxxx http://lists.gluster.org/mailman/listinfo/gluster-devel
