Hacking Code Review (2 Part Series)
Reviewing code requires some original thought. When I read through proposed changes, I think about how the code makes me feel and what my future self might think when I come across it. I spend time collecting my thoughts and feelings and putting them into words that will be helpful and persuasive.
Often though, somebody else has already said what I’m thinking in a better or more comprehensive way. Developers are fortunate that the internet is chock-full of ideas and prior art that we can borrow from. Sharing resources in code review is a great way to make your reviews more impactful without much extra effort.
Sharing API documentation is great for making sure that the reviewer and author are on the same page. It’s easy to develop assumptions about an API’s functionality that may or may not be true, especially as developers become more comfortable with the syntax and conventions. Linking off to documentation ensures that the reviewer and the author have a shared understanding of the APIs they use.
Is this the
distinctwe're using here? Since we want distinct records, I don't think we need an argument.
Blog posts are another helpful resource to share in code review. There are a handful of blog posts that I always refer back to when I bump into a specific kind of problem, like timezones, character encoding, or form UI. Start building up a library of helpful blog posts, and drop them in code reviews when they’re relevant so that you can develop some common knowledge with authors.
I learned in this awesome blog post that
exists?always executes a SQL query. Should we memoize this or consider using
present?to avoid making multiple queries?
Teams make all sorts of decisions that aren’t formally documented. To uncover some of those in code review, I share links to old issues, merged pull requests, and existing code. Through discussions, previous code reviews, and commit messages, I can help piece together the story of why we solved a problem in a particular way and if it makes sense to take that approach again.
I really like the user experience for publishing (#105160), which disables the form and includes an explanatory note for users marked as spammy. What do you think about taking a similar approach here?
Provide context when you share a resource. Dropping a link by itself in a review is cryptic, and it can leave the author guessing what you’re suggesting they do. Authors are more likely to appreciate the resource if you make it clear that your intent is to share something that could be helpful rather than to shame them for not knowing something that you knew already.
VCR has some request matching options that might mean we don't need to get these URLs exactly right to reuse the desired cassettes. I'm wondering if it might be worthwhile to explore that so we can decouple the cassettes from the tests a bit.
Combining a resource with a compliment can be particularly effective. If you appreciate one of the author’s approaches and you’ve seen it executed well somewhere else, share it. It’s a great way to give the author some extra affirmation, and it gives them something to think about the next time they encounter a similar problem.
I love how reusable this new
.text-centerclass is. Have you heard of Tailwind CSS? It’s a whole framework of utility classes like this. Maybe we could consider using it in the future if we like this style.
In code review, a link is worth a thousand words. Save yourself some keystrokes and look for opportunities to share helpful resources.
Cover image by Michael D Beckwith on Flickr.