If you are a software developer or engineer, you most likely have to do code review. At the bare minimum, you probably have had your pull requests reviewed. If you haven’t, then you are probably curious about how the rest of the world deals with the process.

In general, we use code review to make sure we are shipping high quality code that does what it’s supposed to and is easy to maintain. That’s the goal, at least. In practice, code review can get messy. It can be a source of tension between team members. It can be a bottleneck in the development process. But, it’s also a great place for the team to adopt good coding habits and understand how to work better together.

All of that is even more true today thanks to the speed at which we can ship code using AI. The code review process is more important than ever and companies are spinning up to solve that problem specifically. GitKraken is no different and the recent release of GitKraken Review is something you should probably check out.

However, this post is not just some product pitch. We want to go over the “personas” or “hats” that software developers and engineers tend to wear during the review process. No, not hats they literally wear. We’re talking about the thought processes that drive some of the comments we leave and what we look for. This post is inspired by a post I discovered on HackerNews a while back. It’s also a good read for understanding how we wear hats during the dev process as well.

The Librarian

The Librarian focuses on readability over all. They want to make sure that the code is easy to read and understand. They will often comment on variable names, function names, and overall code structure. No single letter variables on their watch. They have written code before and thought “Oh, I’ll remember why I did that” and then came back a month later and had no idea what they were thinking. They want to make sure that the next person who reads the code doesn’t have that problem.

Common PR Comments:

• “Can you rename this variable to something more descriptive?”
• “This variable is shadowed from the context above it.”
• “I love it when booleans are obvious. Could this be ‘isChecked’, instead of ‘checked’?”

The Architect

The Architect focuses on the overall structure and design of the code. They want to make sure that the code is well organized and follows good patterns. They will often comment on the overall structure of the code, the use of design patterns, and the separation of concerns. They want to make sure that the code is easy to maintain and extend in the future.

Common PR Comments:

• “Can you split this function up into smaller functions?”
• “This class is doing too many things.”
• “Could we do this using composition rather than inheritance?”

The Speed Demon

The Speed Demon focuses on performance over all things. They want to make sure that the code is fast and efficient. In the backend, they care query optimization and anything touching “the hot path”. In the frontend, they are almost allergic to unnecessary rerenders. Sometimes, they can be a bit too focused on the details and miss the bigger picture, but they are also the ones who catch performance issues before they become a problem.

Common PR Comments:

• “This migration is missing a composite index on these two columns. Can you add that?”
• “This nested loop could probably be done using an iterator which the compiler can optimize for us.”
• “This useEffect’s dependencies are going to cause a lot of rerenders.

The Tester

The Tester focuses on testing and quality assurance. They want to make sure that the code is well tested and that there are no bugs. They will often comment on the lack of tests, the quality of tests, and the overall test coverage.

Common PR Comments:

• “This function’s use of globals is going to make testing harder.”
• “These selectors should be moved to the PageObject to keep these E2E tests cleaner.”
• “Does this need to be reflected in the OpenAPI spec?”

The Novelist

Documentation is the favorite focus of the Novelist. That could be in the form of asking for comments or making sure docstrings are up to date. They will almost always wonder if the README needs to be updates based on some changes. And they might be the only person who looks at the PR name, and description, and commit messages. But, if they didn’t do it, nobody would.

Common PR Comments:

• “This bit of code is kind of hard to read. Could you maybe add a comment above it?”
• “With this change, the README is out of date. Can you update it?”
• “Can you make sure to add a ‘fixes #XX’ to the PR description so that the Issue gets closed automatically?”

What Did We Miss?

Of course, this list is non-exhaustive. There are many other “hats” that we wear during the code review process. Some people might be more focused on security, others on accessibility, and so on. Most of us will wear multiple hats during the review process. The goal is to understand the intention behind our comments and to effectively switch hats when necessary.

Wrapping Up

We hope you enjoyed this article and that it helps inspire you to think about how you review code and pull requests more. With how much code AI generates these days, the process has become an even bigger bottleneck than it was before. We need to be more rigorous than we ever have before.

Want a Little More Help?

Sometimes you just need a little more help organizing the the code review process. This is where we think you might be interested in GitKraken Review. It’s not here to approve PRs for you, but it does give you all the context you need within a clean semantically diffed interface so you can reduce the noise and review with confidence. It even offers some helpful AI suggestions while keeping the dev in the driver’s seat. Give GitKraken Code Review a try today!