Attention is currently required from: Felix Singer, Nico Huber.
1 comment:
File ichspi.c:
Patch Set #7, Line 872: write_cmd = true;
IMHO, if this was a single "treewide: retype variables as bool where appropriate", I might consider renaming them in a separate commit, but I'd rather do the retyping and renaming at the same time, and make multiple commits for sub-areas. We already have a bunch of commits doing the retyping; if we did the renaming in separate commits, we'd have twice as many commits, which results in too much overhead: more commit messages to write, more work to manually rebase stuff, more commits to review and submit...
If you still want to make separate commits to rename the variables, note that they should be reproducible, so I'd like you to verify that this is the case using timeless builds.
Felix, I can see why Nico's third message may have sounded harsh or otherwise unpleasant, but I'm sure he didn't mean it. I too would be confused after you marked the comment thread as resolved without further discussion, and I also believe that making two commits to change the same lines twice is wasteful. It would've been better if Nico would have explained a bit more why he thinks renaming the variables in the same commit that retypes them. But I know that Nico would've done so, if he had been asked to.
Maybe I'm obsessed with trying not to sound rude, but I would've tried to provide the same reasons but from a "positive" point of view. I feel that "this needs to be fixed/improved by doing X" is a rather negative message, and I much prefer using something like "this is good, but doing X is even better". I believe we should strive for improvement, always trying to make things better. This is how I'd explain my thoughts for renaming in the same commits:
Assuming we agree that renaming the variables is a good thing, doing it in the same commit is less work for the reviewers and less work for the authors as well, if the changes need to be manually rebased (which is usually the case when discussions become sour). The end result is the same (bool variables with meaningful names), but it's less work for all parties involved. It's a win-win situation.
To view, visit change 66892. To unsubscribe, or for help writing mail filters, visit settings.