Where is the value of online diff code review?

Question

I recently joined a team who are using github Pull Request feature as the single code review tool.

In previous teams, I was used to reviews done on a fully checked-out code base, done using IDE tools, code coverage and static code analysis tools, etc.

Compared to these reviews what I get today through github is abysmal. You see basically only the changes and the context is lost. With some efforts you can see a bit more of the file, up to the full file, but I see these code reviews as both very time consuming and of very few benefit.

What is the experience of others using only online reviews ? Did you get any benefit from it ? Was it a frustrating experience like mine ? Did your team eventually evolved to some other type of review ?

I understand I can configure my IDE to do github review from it. But I can't force the rest of the team to do it, merely propose it.

I'm not asking anything subjective. Either a tool has some value in some use cases or it doesn't. Improving the code base or not improving it, catching bugs or not catching them, help sharing knowledge through tools or not help sharing it is factual.

Answer 1

I'm curious how you know that all developers only use pull requests as the only code review tool. On previous teams, the organization's provided a diff-based review tool (like SmartBear's Collaborator and pull requests in Bitbucket), but developers were free to conduct the tool using other methods. In some cases, especially when the reviewer may not have been the most familiar with the code, developers did open their editors and check out different copies of the code, sometimes even running it. Just because it's not spelled out doesn't mean that people don't do it.

The advantage of the diff-based view is that it highlights exactly what the proposed changes are. Tools like GitHub and Bitbucket also allow the pull request author to link the pull request and/or commits to issues in the issue tracker, making them readily available. This gives the reviewer access to a description of the defect or proposed change and any relevant information about it in addition to the change itself.

In my experience, code coverage and static analysis don't need to be done by individual reviewers. CI tools should be executing automated tests, performing linting, and reporting on the health of the codebase. Again, tools like GitHub and Bitbucket can expose links to CI builds and related artifacts in the user interface. If someone has a particular question about code coverage, that information should be readily available.

Generally speaking, though, the diff-based reviews are often sufficient. However, there are different reasons to carry out a code review.

The bulk of my time carrying out code reviews is spent as a knowledgeable person looking for defects or pointing out opportunities to improve the code. If the codebase is well maintained and you have sufficient knowledge, the diff may often be enough to do this. However, when I'm less familiar with the codebase, I'd be far more likely to open the code in an editor and explore, perhaps even running code locally and debugging to observe the changes.

Another factor is the experience of the author. If the author is highly knowledgeable about the work they are doing and comfortable with the language and the codebase, I may trust them more to cover edge cases and not need to do as deep of a dive on their changes as I would be for someone working in an unfamiliar part of the code.

As a team member reviewing code, I'd prefer to not be told how to carry out my review. As a process improvement specialist, the advice that I give is to not be too specific in the details of how individual engineers carry out the reviews. I prefer to establish objectives for the review and make sure that those objectives are met. The objectives would be based on the level of risk of the change, the author of the changes, the reviewers, and the overall intent of the code review process.

So, I'd ask you a question: Are there problems in your code review process? Are there problems in other aspects of the development process that could be solved by adjusting your code review process?

It may be an interesting exercise to work with your teammates to share tips and techniques for good code review practices. However, I'd suggest starting with normalizing on outcomes and expectations for code reviews and how the reviews fit into the overall process so the techniques support the need. Then, each individual should go about conducting reviews in the way that they need to in order to support the goals of the team and organization.

Answer 2

The value of any code review is not nearly as dependent on the tool or the style as on your team.

Different teams have different values and will emphasize different things. So naturally the experience will be different.

What concerns me most is you don’t seem to be getting much out of the experience. That may be because you're getting used to a different style but it’s still something worth talking to the team about. Learn what’s important to them and let them know what you’re not getting.

The code review is where the shop's culture is communicated. Be sure to listen and to add to it. It may be that you have some good ideas that they will want to try.

As for context, once you’re used to the tools you should be able to build that yourself, regardless of style.