| > Instead of making the best attempt to comprehend and embrace the author's style and intention and finding compelling arguments for the question "why this code is probably fine and should be merged as-is?" ... I don't think we should take it as given that the goal should be to approve the code in the originally written form. In some sense, I think if you're in an environment where you're going to be doing PRs for every change anyway, if your code is consistently perfect when you cut the PR, then probably you've cut it too late? What if instead we frame PRs as the basis for an artifact-based discussion about how to do something. The code needs to exist in a complete enough form to provide a concrete basis for discussion. But if you see your reviewer as a resource and collaborator, who might have ideas or insights that you didn't, then ask for those ideas or insights a bit earlier, before the code is polished. The best PRs aren't the ones that catch a bug. The best PRs offer some criticism or question or suggestion that allows you to reframe an abstraction or refactor something to be simpler, more flexible, more testable, more readable, faster, whatever, and from which the original author learns something. Your code might not have had a bug before, but that doesn't mean it cannot and should not be improved. Circling back to 'trust' though, this approach does require a degree of trust. The reviewer can't be focused on nits and gotchas; more minor things are inevitable if our colleagues pull us in sooner, but we have to trust that they'll get sorted out. |