Nature of the beast, I'm afraid. The "beast" is group programming.
First, there ought to be a firmwide linter and style-checker applied to all
changesets. This avoids a whole class of reasons to quibble.
Second, and this is more controversial, outlaw any "this is how I would have done it"
remarks, and institute a policy whereby quibblers make their proposed edits
themselves.
In the vein of "talk is cheap, show me the code," verbal nitting is too easy
and hurts morale. Quibblers, if they feel strongly enough about their nits,
need to make a proper effort to "correct" your code themselves.
My issue with requiring a so-called "quibbler" to develop the proposed change themselves is it puts a high cost on making constructive recommendations.
This can be useful for dealing with the type of argumentative person who will find something wrong with anything. But for a busy person who's been assigned a PR to review, this methodology restricts them from offering feedback since they don't have time to develop fixes, only to suggest them.
Do you resolve this by ensuring PR reviewers are allocated enough time in their schedule to develop suggested changes?
There's another problem here. Requiring others to fix bad code instead of pushing back bad PRs to the original developer removes an incentive for them to write good code. If someone else will fix it, why bother? Do you resolve this at the performance review level?
There ought to be some middle ground here, where you can shut down actual quibblers while allowing legitimate feedback to be quickly given.
> puts a high cost on making constructive recommendations.
A "constructive recommendation" is fine, so long as the reviewer still clicks
"Accept" on the PR instead of "Requests changes." Otherwise, reviewer must
make the change himself. It's called "work" for a reason.
> Requiring others to fix bad code... removes an incentive write good code.
I don't think you understand programmers then. It's an exacting profession --
it's against our nature to embarrass ourselves like that.
I like this solution. It saves a whole bunch of rework. And it forces the person who wants the change (especially after a design review was approved)to put up.
Does it profit anyone to make that distinction? Unless the quibble is a
genuine nit (spelling error or one-line change), have the quibbler implement the desired
change. Oftimes the quibbler misjudges how much code would have to change to
accommodate his "minor quibble." Better to have him figure that out
first-hand, than the original author having to convince him herself.
First, there ought to be a firmwide linter and style-checker applied to all changesets. This avoids a whole class of reasons to quibble.
Second, and this is more controversial, outlaw any "this is how I would have done it" remarks, and institute a policy whereby quibblers make their proposed edits themselves.
I need to use git as an example since that's all I know. Quibblers fetch your PR [following this]( https://stackoverflow.com/questions/27567846/how-can-i-check...), make their changes, and push on another branch, and ask for your counter-review.
In the vein of "talk is cheap, show me the code," verbal nitting is too easy and hurts morale. Quibblers, if they feel strongly enough about their nits, need to make a proper effort to "correct" your code themselves.