Various Notes on Pull Requests and Code Review
Writing and Reviewing PR’s
Chelsea Troy has a few great essays talking about effective pull request reviews, both from the side of writing a good PR:
And discussing the review side:
The post advocates for actually pulling down the code under review, building it, running the tests, and if there’s changes desired, making those changes directly. I think this is an interesting approach and quite different from what I have seen practiced in engineering organizations.
I think that most engineering cultures this would have to be introduced over time, as there’s normally a strong sense that pushing code to someone else’s branch is rude in a sense.
Code Formatting
Another thing that I have found exceptionally useful is to simply totally eliminate the entire class of wasted discussion and comments around formatting, by adopting an idiomatic and highly opinionated code formatter. You can’t please everyone, and I’ve found a direct correlation between code formatters with configurable options, and hours wasted bike shedding precisely what the settings should be.
In cases where I’ve been on a team that’s team to adopted a formatter with little to no configuration like Black for Python, and Prettier for JavaScript, after much initial grumbling, a few months later everyone has forgotten the various objections and are quite happy to no longer be wasting time discussing formatting.