jonathan@researchcomputingteams.org

Category: Technical Leadership: Code Reviews

Parent categories: Technical Leadership

Code Review is Feedback - Linnea Huxford

Code Review is Feedback - Linnea Huxford A reminder that code review isn’t just about the code in question, it’s feedback. So that means it’s an opportunity to give nudges to inform future behaviours (code submissions), it’s an opportunity to give positive as well as negative feedback, and it’s important that all team member are providing consistent feedback.

Continue...

Good Code Reviews

Michaela Greiler on Code Reviews - Software Engineering Radio Episode 400 How to do High-Bar Code Review Without Being a Jerk - Andrew King How to do a Code Review - Google Engineering Practices Documentation Reading Chelsea Troy’s blog has kind of convinced me that Code Reviews are a way of doing asynchronous, distributed pair programming. And even if you do them within an in-person team, they require good communication skills to be productive and drama-free, both in the review itself and “out of band”....

Continue...

Feedback Ladders How We Encode Code Reviews at Netlify - Leslie Cohn-Wein, Kristen Lavavej & swyx

Feedback Ladders: How We Encode Code Reviews at Netlify - Leslie Cohn-Wein, Kristen Lavavej & swyx We had several links about code reviews and the importance of clarity around expectations two weeks ago; in this post, authors from Netlify describe a simple, emoji-encoded 5-level scheme for communicating how urgent and important the code review recommendations are. It’s kind of the code review equivalent of the paper referee’s Reject/Resubmit after Major Revisions/Accepted Pending Minor Revisions/Accepted rubric. Read the article for the details, but the levels are:...

Continue...

The Communicative Value of Using Git Well - Jeremy Kun

The Communicative Value of Using Git Well - Jeremy Kun I’ve mentioned before several of Chelsea Troy’s articles on code review as a sort of asynchronous pair programming, with the benefits both of better quality code and knowledge transfer. In this article, Kun talks about crafting code changes into meaningful commits and PRs exactly to enhance that communication and knowledge transfer.

Continue...

Five Code Review Anti-Patterns - Trisha Gee, Oracle

Five Code Review Anti-Patterns - Trisha Gee, Oracle We’ve talked before about having clear expectations on code review; here’s five common traps to avoid, and that could be made explicit as part of your team’s CONTRIBUTING.md or similar: Nit-Picking Inconsistent Feedback Last-Minute Design Changes Ping-Pong Reviews Reviewer Ghosting

Continue...

What you can do when code is really hard to review - Nicolas Carlo, Understand Legacy Code

What you can do when code is really hard to review - Nicolas Carlo, Understand Legacy Code One distinguishing feature of research software is that it’s often subtle. Subtlety combined with how often it is legacy code makes it difficult to follow, and makes changes doubly so. In this article Carlo describes some general principles for handling hard-to-review code changes, with the caveat that the hard to review changes are the ones that especially need review, both for QA purposes and for knowledge transfer: Focus...

Continue...

How to Make Your Code Reviewer Fall in Love with You - Michael Lynch

How to Make Your Code Reviewer Fall in Love with You - Michael Lynch A nice article outlining how to write PRs to make them as easy review as possible - making them easier to approve. Good for individuals working on open source projects and for teams working together. There are 13 steps there, but several I think deserve calling out: Review your own code first - go through the code with a reviewer’s eyes Answer questions with the code itself - if questions come...

Continue...

Two Kinds of Code Review - Aleksey Kladov

Two Kinds of Code Review - Aleksey Kladov This is another good article of a number we’ve seen here on the topic of code review as asynchronous pair programming, a way of sharing knowledge both ways - about the code itself but also about expectations and goals of the team. From the article: “One goal of a review process is good code.” “Another goal of a review is good coders.”

Continue...

Pull Requests vs (are?) Pair Programming

Those pesky pull request reviews - Jessica Joy Kerr Can pair programming replace code review? - Jonathan Hall Kerr’s blog post in late March kicked off a series of posts in the software-dev blogosphere on whether we should still be doing pull reviews. There’s too many posts to list here, but these two by RCT roundup regulars, cover much of the range of views. Kerr’s pretty firmly on team get-rid-of-’em. My summary of her argument: There’s a reason why no one likes getting or giving...

Continue...

Having a Healthy Pull Request Process for Teams - Alex Kitchens

Having a Healthy Pull Request Process for Teams - Alex Kitchens This is a longer read on setting up a pull request process, both for the authors of the PR and the reviewers. Other processes could be healthy too, but any healthy process will have clear and explicit expectations. Kitchens spells out the responsibilities for an author - they fall under making PRs easier to review: Make the PR Description Clear and Digestible Explain Unexpected Changes Keep the Size of Pull Requests Small (When Possible)...

Continue...

Ship / Show / Ask - Rouan Wilsenach

Ship / Show / Ask - Rouan Wilsenach We’ve talked about pre-commit vs post-commit reviews in #34 - post-commit being something of an alternative to PR review. Changes that past CI testing get committed, so that developers aren’t blocked by waiting for review, and commits are reviewed later. (Obviously this incentivizes a large test suite!) Wilsenach suggests that you don’t have to have a culture where it’s either/or. In the “Ship/Show/Ask” model, changes can be simply made without review (Ship) or post-commit review (or at...

Continue...