Our path so far has been very enriching, we’ve already seen two strategies on how to make atomic commits, and now it’s time to assume the role of reviewers, after all, every software developer is also a code reviewer.
One of the great advantages of having atomic commits is that it makes code review easier, reviewers will thank you for keeping your commits with a single responsibility and organized. Even when we use the all in one strategy, where we have many files distributed in several atomic commits, it is easy for the code reviewer to perform his/her analysis.
As you can see, the divide and conquer strategy makes life a lot easier for reviewers because each pull request has few files, for that reason we are going to use the all in one pull request in this article to demonstrate the power of atomic commits and how to do this review of best possible way.
Our PR looks like the picture below.
We have 8 commits and 19 files to review. If you were to review this PR what would your approach be? Would you go straight to the Files changed tab?
If you went straight to the changed files, you’ve seen a bunch of files that can be scary at first and that don’t clearly state the purpose of each, or the context of each. To make our life easier (and many developers I know have never used this facility) the version control system has a tab called commits and that’s where we’ll do our review.
Take some time for yourself now, breathe and analyze the picture below, see if you can see the beauty and calm it brings us.
As can be seen, we no longer have a bunch of files but an ordered list of commits made, and this order of commits tells us a story, it shows us the entire chain of dependencies starting from the first commit (entities) to the last (root). If you follow sequential logic, you’ll easily see what the system is about, that it has interdependent packages, that it has layers, and you’ll realize that each commit has its purpose.
Reviewing by commits makes life easier for reviewers because they have more control over what they are reviewing, they can stop the review and come back later without worrying about losing what has already been done, or publishing a comment without having finished the entire review, simply because each commit is isolated and has a single responsibility, that is, reviewers can review one by one and mark those that have already been reviewed without losing their revision history.
Enough talking, let’s do the review in practice. Let’s click on the first commit. We will see the following screen.
Wow, much less scary, right?
We only have one file in this commit, so after checking and seeing that we don’t have anything to comment, we can move on to the next commit by simply clicking the next button.
We are now in the commit of the database package, which is the next in our logic and which only contains 3 files belonging to this context, that is, the reviewer does not need to look at all the commits to understand what is being done because he can follow the flow of our application in an intuitive way and seeing only the files related to that moment of the application’s communication flow, which makes the review much easier because it applies the layered review, which is a very good strategy for review.
We can follow this review pattern until the end by clicking the next button until the last commit and pass it if we don’t have any comments. As shown in the figure below, the last commit has the next button disabled.
But wait a minute, in the real world it would be very difficult for a pull request to not have any comments (even being made by the best developer in the world lol), so let’s make a comment and see how we handle it. To do so, let’s go back to the commit of the HTTP package, for that you can click on the prev button until you reach the commit, or simply navigate to the commit tab and click on the HTTP commit. You’ll see the figure below.
Let’s click on the endpoints.go file and put a comment on line 46 as shown in the figure below.
After click in the start a review button we will have the screen below.
Let’s click in the finish your review to finish our review, is always good to send a message to the developer to be polite and a nice guy 😎.
Click in the submit review to see how the PR will look like and we’ll see the figure below.
The reviewer has finished their work, now it’s up to us developers to resolve the conversation started by the reviewer. And now, how would you resolve this issue? The change is very simple, we just need to accept the reviewer’s suggestion by including the variable and changing our function, but the question here is: how will you commit it so as not to spoil the strategy used?
That’s a trick question, isn’t it? What would you do?
Well, the right answer for this is fixups!
