External Publication
Visit Post

Code Review Workshops

~/.bnux June 26, 2024
Source
I've spent the past few months facilitating code review workshops with engineering teams across my organization. Each team had a different review culture, from no formal PR reviews to requiring two approvals before merging. But the friction points that came up in discussion were surprisingly consistent. Here are the patterns I kept seeing. Pull Request descriptions should communicate what, how, and most importantly why a proposed change is being made. The reviewer may not have the full context about the current project or the implementation details. If a change is part of a larger discussion or project, it's helpful if the PR author includes a summary and links to the project or discussion. Leaving some breadcrumbs can be useful in the future for developers who maintain or expand the feature. PR templates can serve as valuable tools for structuring PRs to ensure essential details are not overlooked in the review process. Be careful, however: a heavy PR template often leads to teams ignoring most of it over time. PR templates should be curated so that they maximize usefulness in the majority of cases without trying to cover every possible scenario and making it hard to parse what's important. This applies to commit messages too. Your commit history tells a story about how and why changes were made, and reviewers can use it to understand the order in which changes were implemented. At minimum, a commit message should explain why a change was made rather than simply describing what was changed. A sync chat can save days of back-and-forth. Sometimes a PR comment thread starts to feel like a slow-motion tennis match. You leave a comment, wait for a response, respond back, wait again. Before you know it, days have passed and you're both frustrated. If you find yourself in a lengthy back-and-forth, consider jumping on a quick call. A 15-minute conversation can often resolve questions and concerns that would take multiple days of asynchronous discussion. You can always document the outcome in the PR afterward. This is especially true when: The intent behind a suggestion isn't clear You're discussing trade-offs between different approaches There's confusion about requirements or implementation details Emotions are running high and tone is getting lost in text Don't be afraid to add a comment that simply says "Let's chat about this quickly, when are you free for a 15-minute call?" Share suggestions clearly, kindly, and with curiosity. How you deliver feedback matters as much as what you're flagging. Frame suggestions as questions when appropriate: "What do you think about extracting this into a helper function?" invites discussion more than "Extract this into a helper function." Lead with curiosity rather than criticism. Instead of "This is wrong," try "I'm not sure I understand this approach, can you walk me through your thinking?" In one workshop, a team shared that they'd historically skipped PR reviews entirely because the volume of work made it feel impossible to keep up. When they started introducing reviews, they found the biggest value wasn't catching bugs, it was learning from each other. That reframe helped the team see reviews as an investment rather than a bottleneck. Be explicit about what's blocking versus what's a suggestion. Distinguish between: Blocking: "This will cause a security vulnerability and needs to be fixed before merging" Non-blocking: "We could simplify this with a instead, but the current approach works fine" And remember to point out what's done well. If someone wrote clear tests, refactored something nicely, or handled an edge case you wouldn't have thought of, say so. Positive feedback is just as important as constructive criticism. It's also worth building the habit of looking for what's not there. Reviewing what's in front of you is the easy part. The harder part is asking yourself what's missing. Is this validated? Is it tested? Are we handling errors? Is there an accessibility or security concern? You have to train yourself to look for gaps, and most of us haven't. Review your own PR before requesting review. Not in your editor, where you've been staring at the code for hours. On GitHub, in the PR view, where it looks different and you're seeing it the way your reviewer will see it. That context switch is surprisingly effective at catching things you missed. Some people go a step further and leave comments on their own PR, explaining decisions they know will raise questions. This saves the reviewer time and shows you've thought through the tricky parts. It also shortens the review cycle because you've preempted the most obvious round of questions. Periodically review the review process itself. Code review practices should evolve as your team and codebase evolve. Set aside time quarterly (or whatever cadence makes sense) to reflect on and improve your review process. Some questions worth discussing: Is this PR checklist useful? Are people actually using it, or just checking boxes to get through the process? Are we paying attention to these tests and bots? If CI checks are consistently ignored or overridden, maybe they're not providing value. How do we approach large PRs? Should we break them down differently? Review them in stages? What's our standard for "done"? Are reviews taking too long? Getting rubber-stamped? Finding the right balance? The goal isn't to create more process for process's sake. It's to ensure your review practices are actually serving your team and helping you ship better software. One more thing worth paying attention to: silence. If your PRs consistently get clean approval stamps and no questions, that's not necessarily a sign that everything is clear. Sometimes it means people don't feel comfortable asking. A review process with no pushback and no curiosity might look efficient, but I'd be more concerned than reassured. Final Thoughts There's no one-size-fits-all approach to code reviews. The best practices for your team depend on your team size, codebase, culture, and the types of problems you're solving. But at their core, effective code reviews are about communication, collaboration, and continuous improvement, for both the code and the people writing it. These workshops have reinforced that code review is a skill worth developing intentionally. Catching bugs matters, but code reviews also create opportunities for knowledge sharing, maintaining code quality, and helping each other grow as engineers.

Discussion in the ATmosphere

Loading comments...