3 ms·

Because your change and the formatting change are probably dependent, and between the time you submit the change and the time your PRs are accepted there may be

Because your change and the formatting change are probably dependent, and between the time you submit the change and the time your PRs are accepted there may be multiple other commits to that file which affected lines that your formatting PR addressed. If changes wait for weeks to be merged (which is actually pretty quick compared to years) you wind up with the formatting change being blocked on needing a rebase. The thing becomes an annoying PITA of merge conflicts and rebases. So either have code formatting standards to start with which are automatically applied, or else accept PRs that have whitespace fixes and merge them promptly (I can accept splitting up the commits into formatting and non-formatting commits but even that is barrier to entry over people being able to submit changes to your software easily--every bit of fussiness is a tax on the submitter's time and will result in less submissions).

saurik 3y ago
You make it sound like both of these changes have to be entirely separately reviewed, but most of the code review processes I have seen--including mailing lists--have the ability to merge in a sequence of patches in a single go, rather than having to review each one by one. Otherwise, if the problem is that you have patches waiting to be merged for weeks that result in merge conflicts, that isn't solved by putting the patches together into a single patch: you just have a process that is going to make formatting changes impossible to clear without tons of extra work. As for submitters, they need not put in this time: the maintainer or review team can trivially split the patch as this is literally close to hunk selection... it is so easy that I routinely do this as part of every single commit I have made even locally and even temporarily for as long as I can remember? Regardless, I still don't understand: nothing you are saying makes it sound like the people in your scenario are actively making code changes to avoid having to make a formatting change--"a tactic to continually change code to get away from having a formatting change come in--which is what was claimed and what seemed so utterly confusing... I can't imagine someone saying "I'm going to change this logic few days as I hate formatting patches".
- lamontcg 3y ago
  You're assuming a relatively high degree of competency and available time on the part of maintainers and it sounds like a reasonably short backlog. That's great. For overworked, understaffed teams like a large backlog of reviews it generally doesn't work like that. And for those teams the work done on fixing the formatting issues across their project pays off in velocity because submitters and reviewers stop needing to care about the issue entirely. This is why when people think about submitting a change and they see their editor auto-formatting changed half the lines in the file with whitespace fixes, they get a bad feeling about how submitting anything to the project is going to go.

saurik 3y ago

You make it sound like both of these changes have to be entirely separately reviewed, but most of the code review processes I have seen--including mailing lists--have the ability to merge in a sequence of patches in a single go, rather than having to review each one by one. Otherwise, if the problem is that you have patches waiting to be merged for weeks that result in merge conflicts, that isn't solved by putting the patches together into a single patch: you just have a process that is going to make formatting changes impossible to clear without tons of extra work. As for submitters, they need not put in this time: the maintainer or review team can trivially split the patch as this is literally close to hunk selection... it is so easy that I routinely do this as part of every single commit I have made even locally and even temporarily for as long as I can remember? Regardless, I still don't understand: nothing you are saying makes it sound like the people in your scenario are actively making code changes to avoid having to make a formatting change--"a tactic to continually change code to get away from having a formatting change come in--which is what was claimed and what seemed so utterly confusing... I can't imagine someone saying "I'm going to change this logic few days as I hate formatting patches".

lamontcg 3y ago
You're assuming a relatively high degree of competency and available time on the part of maintainers and it sounds like a reasonably short backlog. That's great. For overworked, understaffed teams like a large backlog of reviews it generally doesn't work like that. And for those teams the work done on fixing the formatting issues across their project pays off in velocity because submitters and reviewers stop needing to care about the issue entirely. This is why when people think about submitting a change and they see their editor auto-formatting changed half the lines in the file with whitespace fixes, they get a bad feeling about how submitting anything to the project is going to go.