[PATCH v2] user32: In menu.c converted all tabs to spaces

Michael Stefaniuc mstefani at redhat.com
Thu Feb 2 05:36:07 CST 2017


On 02/02/2017 11:12 AM, Fabian Maurer wrote:
> On Thursday, February 2, 2017 9:30:49 AM CET Michael Stefaniuc wrote:
>> sorry, but this is just change noise.
>>
>> You can do the TAB to SPACE changes en passant when modifying the
>> respective lines.
>> Probably the only valid exception is when the indentation of the code is
>> visibly messed up. But that's not the case here.
> 
> Well, there's really a lot of tab and space mixing,  it's a lot cleaner to use 
> a consistent style.
Right, for new code.

But how is that a problem for old code? The code displays normally thus
it isn't an issue for the human reader.
While it is visible in a diff it is most of the time just an off by one.
And a hint to clean up the TABs in the modified lines.


Now to the downside of such massive cleanups:

- Time to create and submit this patches. Can be probably ignored; it's
open source volunteer work so everyone is free to spend his/her time at
will.

- Time to review the patches. This is a bottleneck.

- It makes it harder and slower for humans *and* machines to track code
history. This is the most important reason.

* Time wasted with git blame backtracking where the change came from.
Using -w is not a good default option as there might be relevant
whitespace changes when the indentation level changes, e.g. code moving
in or out of an if block.

* Conflicts when cherry-picking patches for Stable.

* Conflicts when rebasing Staging patches.

Those aren't hard problems for a human but they waste unnecessary time
of volunteers.

Two machine use cases of git blame that get harder:
* git deps (https://github.com/aspiers/git-deps). I use it extensively
for Stable. Such a big patch as a dependency can make all the difference
in git deps finishing in seconds or at most a few minutes to 1+ hours.
The length of the generated dependency list plays also a small role in
deciding if a patch is a Stable candidate.

* PVS-Studio defect reports. PVS-Studio is a commercial software and the
vendor has run the scan and provided us the log. They do that for free
so we didn't want to waste their time asking for frequent reruns. So
I've used git blame in a script to track the movement of the line number
of defects. That allowed both Nikolay and me to fix a lot more defects.
Also when git blame marked a line as modified I could review it and mark
it as solved.


So this is the reason why Wine has a policy to disallow massive style
changes. Very little benefit, but significant costs.

bye
	michael



More information about the wine-devel mailing list