In Progress
Unit 1, Lesson 1
In Progress

Breaking Down Complex Pull Requests with Claudio Baccigalupo

Having a robust code review process is a proven way to catch bugs before they hit production, as well as to spread preferred practices throughout a team. But speaking for myself, nothing makes my eyes glaze over quite like a five-hundred line commit ready for review. In today’s episode, Claudio Baccigalupo shows you how to break down large pull requests into small, granular commits. This makes reviewing code easier, and leads to a higher-quality code review process. Enjoy!

Video transcript & code

Breaking down complex pull requests

Have you ever been assigned a Pull Request to review and…

…once you open it, you are not sure where to start? You find multiple commits with several line changes split across files.

You try to wrap your head around the problem but it's hard even to know where to start.

If you have ever been on that side of the review, you know it can be hard. Let's see how we can simplify the work of a reviewer by submitting pull requests that are easy to understand and digest.

Let's take for example this basic Rails application I have created. It's a blog app with two posts.

Clicking on a post…

…shows the content of the post and…

a link to leave a comment.

After you enter your name and a comment…

and you submit the form…

…a confirmation message informs you that the comment was created. But wait… we forgot something! There is no way to read the comment. We are missing a page to see all the comments of a post so… let's build it!

This is what the site looks like after adding the feature. Each post has now a link called "Show comments" and clicking on the link…

…reveals the comments left for that post. This is the feature we want to add.

Now, what code did we have to write for this feature?

Let's look at the result of git diff:

This diff is not gigantic, about 20 lines of code. Still, there are changes that someone who has not written the code might wonder about.

For instance, if this feature is just about showing comments, why are we even touching the new and create actions? The change is subtle: we are extracting duplicated code into a private method. We don't need this change for our feature, but it will improve the codebase. So what is the best way to communicate this to a reviewer?

My answer is: extract these changes in a separate commit. Have a first commit that only focuses on removing duplicated code, and a second commit with the actual feature.

To selectively add only part of your code change to a commit, my favorite way is to use git add with the --patch option. This will go through every single block of line changes and ask you whether you want to include that change in the next commit or not.

Here is the first block of line changes. Since we only want to stage some of these lines in the first commit, we type "s", which stands for "split", to break down the changes into smaller blocks.

Now this is definitely a change we want in the first commit, so we type "y" to include it.

We continue like this, type "y"…

...for each change we want to include...

...and typing "n" for changes we want to exclude…

… and finally typing "q" when we are done.

We can now review all the changes we have staged…

…and confirmi that they only deal with removing duplicated code.

Time write our first commit!

To help the reviewer understand what this commit is about, my suggestion is use terms like "Better code" or "Refactor".

Remember to always limit the first line to 50 characters and to add a blank line after that. And don't be afraid to be super-verbose.

Now that the first commit is done, we can stage everything else.

 (No text between slides)

Check how this second commit is now clear to understand:

there is a new link to "Show comments" and the minimal route, action, and view to display them. Time for another commit.

In your commit messages, don't just describe what you have done, but why.

Why are we adding this code? Because previously there was no way for users to see their comments.

To conclude, we now have two commits that have value on their own.

Once we create a PR, the reviewer will be able to read them separately and suggest changes to one or the other.

GitHub has a great interface to review PRs commit after commit.

Thanks for watching this episode. Remember: programming is not just about writing code is also about communication. Breaking down a pull request can go a long way in helping others understand your code.

The smaller your commits, and the more descriptive your messages, the easier will be for a reviewer to accept your changes. If you have never used git add with the --patch option, give it a try today.

Happy hacking!