Comment by greenthrow
Comment by greenthrow 5 days ago
I don't disagree some people have bad, overly nitpicky PR habits and they should be trained out of that.
That said, sitting down before hand and planning is useful for the big picture but one of the big benefits of PRs, as I explained earlier, is to eliminate lots of wasted time going back and forth in meetings about how to implement something and instead empowering engineers to go write code and then to iterate on actual implementations instead of endlessly discussing ahead of time. So there is a balance there where you make sure everyone is aligned on what you're building but also being open to suggestions on your PRs. Sometimes that may mean rewriting large chunks in exchange for drastically reducing pre-coding discussions.
I agree that balance is necessary and nitpickers should be steered away from that behaviour.
I think I'm more concerned with nitpickers which show up in different flavours.
I understand a functional requirement to rewrite a piece of code in a PR. Perhaps you spent the last 8 hours coding up a solution that passes all of the tests but if you use that data structure at scale it wouldn't perform acceptably. That's a rewrite and that sucks but it will be for the better.