Hello Fabian,
On 02/01/2017 11:25 PM, Fabian Maurer wrote:
This change makes it a lot to edit easier with some editors. Conversion done with Notepad++, with tabsize 8 spaces.
v2: removed trailing whitespace
Signed-off-by: Fabian Maurer dark.shadow4@web.de
dlls/user32/menu.c | 1922 ++++++++++++++++++++++++++-------------------------- 1 file changed, 961 insertions(+), 961 deletions(-)
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.
bye michael
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.
Regards, Fabian Maurer
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
On Thursday, February 2, 2017 12:36:07 PM CET Michael Stefaniuc wrote:
So this is the reason why Wine has a policy to disallow massive style changes. Very little benefit, but significant costs.
I understand, thanks for the in-depth answer. I really didn't want to cause more work, just figured it would help if the code was cleaned up a bit. But I guess you have a bunch of good reasons that I failed to consider.
Regards, Fabian Maurer
-
Fabian Maurer -
Michael Stefaniuc