The following is a transcript of the latest episode on the Professional JavaScript podcast.
Tagline
Hey everyone and welcome to the fifth episode of Professional JavaScript, the podcast for learning how to grow as a JavaScript developer in a professional environment where the problems are greater than just another todo list tutorial.
Each week, I, Michael Mangialardi, will be sharing what I’m learning as I write JavaScript code on a professional team. I’ll cover tips, tricks, technologies, and other goodies that you don’t always get when consuming other online resources.
Today’s Episode
In today’s episode, I’ll be discussing how to help someone else review your code.
Introduction
You are bound to find many articles floating around in the blogosphere about how to conduct a code review.
That is great as learning how to conduct a code review is a valuable skill that is not always acquired before acquiring a junior developer role.
Perhaps I’ll record a podcast on that topic one day.
Today, however, I want to talk about how you can help someone else review your code.
Let me explain.
One of the many buzzwords that you’ll hear in developer circles is “context switching.” This word does have a technical meaning and origin.
According to the scholarly source called Wikipedia, context switching can be defined as:
The process of storing the state of a process or thread, so that it can be restored and resume execution at a later point. This allows multiple processes to share a single central processing unit (CPU) and is an essential feature of a multitasking operating system.
However, the term “context switching,” in most cases, has been adapted by developers to describe a common challenge to their productivity.
Context switching, in this sense, refers to the switch between tasks throughout your day.
For example, your day as a developer may look like this.
8:30 AM - Login and check email and Slack
8:45 AM - Catch up on code reviews
9:30 AM - Attend the team standup
9:50 AM - Work on a new feature
1:30 PM - Attend the design-dev collaboration session
3:00 PM - Continue to work on a new feature
3:05 PM - Response to a co-worker’s question
…the list goes on
The point is that your day can be full of switching from one context to another. Sometimes these switches can be predicted (like a formal meeting). Other times, they are spontaneous (like a co-worker messaging you on Slack).
Context switching tends to not be a developer’s favorite thing.
Given that having someone else review your code imposes a context switch on them, it is nice to help make that context switch as smooth as possible for them.
So, how do you help make their context switch smooth? How do you help someone else review your code?
Body
Let’s start with the first thing that will grab the reviewer’s attention, the pull request title.
Pull Request Title
Your title should make clear as to what will happen if your code is merged. It is best, when possible, to phrase this from the perspective of a user who would want to know what new feature/functionality is available should the code be merged.
If your repository is a user-facing application, the “user” whose perspective you are trying to write from would be the end-user/customer.
For example, imagine you just fixed a bug. Before, your users weren’t seeing highlighting on a screen as they typed in the search bar. Now, they do see highlighting as they type.
A bad pull request title may describe the technical solution rather than the functionality the end-user would see should your code be merged.
Example:
Include Highlights in Search Action
Another bad name is when the impacted feature is mentioned without mention of how it is being impacted.
Example:
Bug Fix: Search Highlighting
A good name would be:
Highlight Catalog Search Matches As User Types
Let’s break down this good name.
First, it begins with a verb (highlight).
Second, it contains the context about the part of the application that is being touched from a user’s perspective (catalog search matches).
Third, it appends additional context to describe the user’s interaction (as the user types).
If your repository is tooling for other developers to consume, you would write from the perspective of a developer who would be consuming the tool.
Imagine you’re adding EsLint to a GraphQL server’s repository.
A bad name would not begin with a verb nor capture the developer’s experience.
Example:
EsLint Support
A more helpful title would be:
Enforce JavaScript Syntax During Local Development
It’s not wrong to include the added/impacted technologies as long as it adds to (rather than taking away from) the helpful title:
Enforce JavaScript Syntax During Local Development Using EsLint
Now, there is no way I can iterate through all the example scenarios and the appropriate title. Hopefully, this gives a framework for improving them.
Even if you do not enjoy this particular framework, merely putting effort into consistent, helpful pull request titles will go a long way.
Similar to a pull request title, pull request commits should be helpful and have intentionality behind them.
Pull Request Commits
First things first, let’s talk about the commit history.
A good pull request title helps the reviewer know the end goal of the committed code.
The individual commits unravel the “story” of how the developer solved the problem and achieved the result mentioned in the title.
These commits record the logical progression from a problem to a solution.
In the German fairy tale Hansel and Grettel, Hansel leaves breadcrumbs to be able to find his way home.
The table of contents of a non-fiction book is effectively the “breadcrumbs” of how a message is communicated. Each chapter title summarizes what to expect in the chapter. Each chapter title highlights the order of the book and its logical progression to communicate a message, culminating in a conclusion.
The commits of a pull request are the “breadcrumbs” that reveal the logical order of how a solution was attained, culminating in the result encapsulated in the title.
One extreme of a commit history is to leave a breadcrumb at every step:
Refactor function
Remove semi-colon
Update title
Another extreme is to leave out helpful breadcrumbs that tell the reviewer your intentionality:
Fix search highlighting as user types
Format code
These are the two extremes, and you will have to find the middle-ground. Instead of emphasizing what you should write, let me advise what you should ask.
Stop and ask before you tag a reviewer:
Does this commit history tell the story of how I solved the problem and reveal my intent?
If it doesn’t make sense to you, it will not make sense to a reviewer.
Of course, the best time to be thinking about how to structure a commit history is as you are developing. Mere intentionality to do this will go a long way.
Don’t be afraid to rewrite your git history. I have to do this in almost all my pull requests.
Second, let’s talk about the naming convention for commits.
A classic recommendation is to use Tim Pope’s Git Commit Message Template:
Capitalized, short (50 chars or less) summary
More detailed explanatory text, if necessary. Wrap it to about 72
characters or so. In some contexts, the first line is treated as the
subject of an email and the rest of the text as the body. The blank
line separating the summary from the body is critical (unless you omit
the body entirely); tools like rebase can get confused if you run the
two together.
Write your commit message in the imperative: "Fix bug" and not "Fixed bug"
or "Fixes bug." This convention matches up with commit messages generated
by commands like git merge and git revert.
Further paragraphs come after blank lines.
- Bullet points are okay, too
- Typically a hyphen or asterisk is used for the bullet, followed by a
single space, with blank lines in between, but conventions vary here
- Use a hanging indent
I would second this recommendation.
I would add, however, that what a summary should be similar to a pull request title, except you are writing from the perspective of how the reviewer will understand the code changes.
I would also clarify that the context of the impacted code (relative to the application and the codebase) is less important as that can easily be seen when the reviewer checks the diff for that commit.
Moreover, do not abuse the longhand, explanatory text.
A bad example of explanatory text is when it is used in place of a git commit history:
Fix search highlighting as user types
1) I did X...
2) I did Y...
...
Also, the explanatory text does not need to describe everything that your code does and its context relative to the codebase. All of that can be discerned by the reviewer when reading the diff for the commit. However, being verbose in this case does not harm; it just adds more work.
Practically, I have tended to use explanatory text for details that may not be obvious from the code diff. A good example of this is an external link to a Stack Overflow answer, package release changelog, GitHub issue, etc.
Working to organize your commit history with good naming conventions will significantly help your reviewer to understand what is going on in the pull request. They will thank you as it helps them switch context with greater ease.
Pull Request Description
Finally, the pull request description should contain any additional context that would help the reviewer to understand what is going on as they switch context.
Now, every team will have different criteria as to what should be included in the description.
At a minimum, it should include the what, why, and how behind the changes.
It should call out and provide links to any supporting pull requests.
Commonly, it may include a link to the story/card/ticket that may exist in a tool like Jira or Asana as created by a dev manager, tech lead, or technical product manager as well as the acceptance criteria (the requirements that were specified as to what needed to be done).
You should also provide screenshots that show that you have manually tested your code, effectively allowing the reviewer to get a visual of the pull request title.
Conclusion
Being intentional about helping someone else to review your code will not only assist them as they switch context, but it will also mature your skills as a developer to think and plan as you code solutions.
It can also help to improve your technical writing skills which is a tool to have in your toolbox.
My hunch is that this advice will apply mostly to junior developers.
If you’re a junior developer, this will help progress as a more mature asset to your team.
If you’re applying to be a junior developer, then having knowledge of git and constructing good pull requests is a soft(ish) skill that can give you an edge over other applicants.
Consider taking the time to learn git and practice shaping a good commit history and pull request for an open-source project that you can include in a portfolio.
At the end of the day, however, every book has a table of contents despite the writer’s maturity. Hopefully, this provides helpful advice for all developers.
