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
- Splitting the PR in meaningful commits.
- 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
- Realizing if I left some shortcomings and removing them.
- 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.
- 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
- Rebase the branch over the
main
one - Push the branch to
origin
(allows me to revert to the original branch in case of big mistakes) - 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:
-
git rebase -i
and some manual changes. - 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...
- 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".
- 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.
- 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.
- Whatever: usually, a bunch of
git revert
s (through the SmartGit UI since it's easier for me) andgit squash
(throughgit 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:
- 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.
- There are more commits than files in this branch: sure! But I care about the story, not the number of commits.
- 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
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.You mentioned stacked PRs, which is hard to deal with. Are there any tools you use for this purpose?
Yes, Graphite.-
Do you have published other articles on the topic?
Yes, before this one, you can find:
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
- Doing sync code reviews instead of async ones, so sharing the knowledge is faster.
- 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.
Top comments (0)