Comment by agentultra
Comment by agentultra 5 days ago
I realize my comment was emotionally charged.
The point was that I'm a planner. I tend to discuss designs long before I begin writing code. By the time I get to a PR I don't expect to get feedback about the design. I'm looking for someone to check that the code is acceptable to merge. If you have problems with the design, it is too late!
However I work with folks who are not planners. They will write a PR as a proposal. Their expectation is that they may need to iterate on the code a bit based on that feedback; even the design and approach itself is up for review. If you have problems with the design, it's fine!
What has worked well for me in those situations in the past is to be up-front about what kind of feedback you're seeking. Mention in the PR description that you're looking for design crit, code review, etc. Whatever lingo works for your team.
Update: Realizing at the last minute that the design itself is wrong, and saying so, is a good thing -- and I do appreciate that. However it should be rare. My pet peeve is more with the folks who nitpick... whose design contributions are superficial and concerned with style more than substance.
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.