Re-building a branch and telling a story to ease the Code Review

What tools I use and how I proceed when I am done with the code and I am about to open a PR? I illustrate a use case of the whole process.

Photo by Erik Mclean on Unsplash

---

After completing a feature and before opening a PR, I tend to destroy the branch and re-create the commits from scratch. The main goals are

1. Splitting the PR in meaningful commits.
2. easing the reviewers digging into my code commit by commit, reading the incremental information to understand what I did.

There are a lot of secondary advantages too, like

1. Realizing if I left some shortcomings and removing them.
2. Double-checking if the choices I made when I started working on the feature are still valid at the end when the context is more apparent.
3. Realizing if the PR is too big, and split it into more stacked PRs if needed.

But the main goal is to ease the code review process as much as possible. In the next chapters, I am going to show how I do it by taking the best out of a bunch of Git-related tools: the standard Git CLI, SmartGit, and Lazygit.

The use case

I just finished building an automatic recorder of the request/responses for the Hasura's E2E tests. As you can see, there are many "WIP" commits (screenshot of the SmartGit log).

The goal is re-building the commits from scratch, resulting in something like

Resetting the branch

Resetting the branch is the easiest part, I usually

1. Rebase the branch over the main one
2. Push the branch to origin (allows me to revert to the original branch in case of big mistakes)
3. Reset the branch through git reset main

Then, all the commits of my branch got removed. I have all my changes in place, ready to be committed (like if I had coded without never committing anything).

Committing files and patches

If you have not used them previously: thanks to Git patches, you can stage and commit only a portion of the changes you applied instead of staging and committing whole files. Lazygit makes working with patches freidly. Thanks to Lazygit, you can scroll into a file with your keyboard and add only the lines you want (see the "Staging lines" part of the official video).

I tend to add files and/or lines of code based on the story I want to tell to the reviewer. In this phase, I create quick commit messages that allow me to return on them later, squashing/moving them. In my experience, a lot of times, I forget some details for the non-last commit; hence I cannot git amend it anymore (for instance: when I commit a function and the code that consumes it... but I did not commit its export index.ts file). Once I have all the quickly-created commits like the following ones

I need to move/squash them... (go to the next chapter)

Fixing the re-built history

When I need to re-organize also the re-built branch (it happens a lot of times to me) I opt for:

1. git rebase -i and some manual changes.
2. SmartGit's "Move commit" and "Squash commits" feature

Here I highlighted the commits I marked as to be squashed at in the previous step

Enriching the commit messages

Now that the commits are ok, I edit their messages one by one to add all the details that the headline cannot tell. For this purpose, SmartGit's "Edit commit message" is my tool of choice (since I am not a Vim champion).

Double-checking the commits

What can go wrong with all the previous steps? Well...

1. I could realize that I should split some commits into more commits. To fix this and also avoid creating a Git mess, I use SmartGit's "Modify or Split Commit".
2. I could realize that some commits should have been ordered differently. This is the worst situation because fixing it can be a mess because of the heavy usage of Git patches... Here I decide if leaving with the wrong order or almost redoing anything from scratch (😱). The original pushed branch is here to help me.
3. I could realize that I made a mistake in the PR and I want to fix it. Well, similarly to the previous point... I live with it and I add the commits at the end most of the times.
4. Whatever: usually, a bunch of git reverts (through the SmartGit UI since it's easier for me) and git squash (through git rebase -i) allow me to fix the mistakes I did.

Opening the PR

What remains in the GitHub issue only are all the media files I attach to the PR. Usually I add screenshots to explain the context/outcomes in a while, videos to explain what I did and to avoid the reviewers running anything on theirs machines... Unfortunately, these media remain part of the PR's documentation.

Et voilá, I am done! This is the screenshot of the PR, and this is the final commit history 😊 (screenshot of the SmartGit log)

Please note:

1. If you are wondering what the "(TEMPORARY COMMIT)" are: they are commits that help me tell the story. Then, I removed them at the end of the history. If the reviewers do not go commit by commit, they do not see them nor the included changes.
2. There are more commits than files in this branch: sure! But I care about the story, not the number of commits.
3. The commits will be squashed after the PR is approved. In other cases, I value in keeping the commits as they are.

How long this the whole process take? In the case above, it took 30 minutes for me. Improving also the code and describing the PR took 2 hours to me.Please consider that I tend to weigh more on how much time I save for the reviewer and how short the time my PR stay open.

If you have tips to share, if your flows are more productive than mine, please let me know!!! 🙏

FAQ

1. Is there a way to reduce the amount of time dedicated to this kind of activity?
   Sure, the better your analysis is, the less time you dedicate rebuilding the branch. In the above example, I needed to test different combinations and outcomes before choosing the path to follow.

2. You mentioned stacked PRs, which is hard to deal with. Are there any tools you use for this purpose?
   Yes, Graphite.

3. Do you have published other articles on the topic?
   Yes, before this one, you can find:

   • Support the Reviewers with detailed Pull Request descriptions
   • A Case History: Analysing Hasura Console's code review process
   • How I ease the next developer reading my code

Feedback

I share all the meaningful feedback I receive from the friends and readers.

Simone D'Avico

30 minutes are a lot, you can reduce it by

1. Doing sync code reviews instead of async ones, so sharing the knowledge is faster.
2. Applying this "Ship / Show / Ask" approach.

From various Hasura's front-enders

The approach is interesting, as GitHub shows all the commits with a comfortable "Next" button.
The commit message and explanation could avoid the reviewer from spotting code that is not clean and readable, just because the commit description explains it.

I think having commit messages to explain the workflow is super useful if I was doing a functionality review simply because I'm going through one small change at a time making it easier to think about its impact.

In general, I think I would still look at the whole PR changes, rather than navigate through the single commits.