Git Gud—an opinionated Git styleguide

All opinions are my own and not those of any employer past or present.

This is a styleguide on how to use git, the version control software. It defines a set of rules to follow when cooperating on shared code bases, provides reasons for each rule, and deliberates on the compound effect of following all of them to the letter.

An effort has been made to divide the styleguide into sections that lend themselves well to be read from top to bottom, but also to split them with enough granularity to quickly find a specific rule, and with clear delineation between overarching topics. All of the sections that seem likely to be referenced when discussing rule violation are numbered, and the rest are not. Whether numbered or not, every section can be deep-linked to by copying the header link.

All commits used as examples, good or bad, are purely fiction. I dissect them as if they were not for the sake of argument, but they are.

I have no doubt this styleguide works.

It works every day, in fact, in time of writing at my current team at work. Specifically it works by affecting the benefits alluded to later in the guide at the cost of potentially spending more time writing commit messages. However, it only works when it is actually followed. This may seem obvious, but please don't forget it.

Throughout this styleguide the branch name “main” is used to refer to the default branch. You may prefer “master”, which is becoming more controversial by the day; “develop”, which seems to suggest other branches don't enjoy development; or whatever else has your fancy. If you want a recommendation: just use “main”. If you feel strongly about some other name: so be it. Recognize, however, that it's probably not worth fighting this particular tide, which seems to be sweeping “main” across the landscape in time of writing.

If you are at all able to choose your default branch name, pick “main”. If you are not able to make such prosaic decisions as default branch name, stand up to the oppression. If you lost your fight for freedom, fret not. This failure should not break the usefulness of the styleguide as a whole.

A few overarching principles have shaped the rules of the styleguide. Make sure these align with the team values you intend to nurture.

1. Conversation is key—the foundation for code quality, knowledge sharing, and building a sense of shared ownership is to get team members talking about how and why their code changes. It is essential to foster a review culture where people feel comfortable sharing and receving feedback with each other in constructive ways.
2. Short iteration cycles—the duration between picking up a task and merging it into “main” should be as short as possible. This may be harder than it sounds, but it is crucial for this styleguide to work its best.
3. No long-lived branches—the number of branches with long-lived semantics that have to persist for an extended period of time to follow the styleguide must be kept to a minimum. This styleguide defines that minimum as one long lived branch: “main”. The styleguide focuses strongly on simplicity in workflow and does away with some of the complexity of older branching guide classics by not using several persistent branches as the mechanism for releases, hotfixes, and non-production environments.
4. Establish a base line of mandatory information—whether you're reviewing a code change, trying to understand an old commit's purpose, or drafting up release notes, it's a major help if you can trust certain information to be available. This styleguide treats Git as a professional tool and expects its users to act as professionals.

The styleguide assumes your team is already doing well. If your software development process is not a well-oiled machine, this styleguide might help in some small way, but you probably have other more important improvements to pursue that could prove more impactful relative to effort.

Hopefully you're here because you care how your team uses version control software for their source code and want to improve some aspect of this practice. You would like to see it streamlined with sensible rules that all are required to follow like in so many other aspects of professional work life, and you want to lean on a battle-tested foundation.

In order to impose changes on team practices you will likely need some measure of persistence, fervor, and perhaps even authority. You may find that people don't like to put effort into a cause that has been forced upon them, and that the cause needs a zealous champion to succeed. You will most likely have to be that champion.

Alternatively you're here because another such zealot strong-armed you. If so just skip to the good parts as it seems I've successfully outsourced the task of forcing you into acquiescence convincing you about its merits.

This document is intended for someone who:

• is part of a team of software developers cooperating on a shared code base.
• intends to and is capable of applying this styleguide wholesale, unflinchingly, and without compromise.
• considers their software development team well functioning and already delivering software competently.

It might not provide its purported benefits to someone who:

• fails to convince all their team members to follow it rigorously.
• experiences major struggles with other parts of their software development process.
• can not muster the energy and empathy to show their team members the ropes, over and over until they see the light.

Perhaps this all sounds quite daunting. Are the rules really complex? Is this some sort of extremely advanced collection of Git tricks for the secret software elite, then? A list of unholy Git arcana that only the highest echelon of hackers may attain?

No. This really is just a styleguide. It lays down some simple, enforceable rules and helps well-oiled teams accomplish some specific goals in their cooperation with each other.

At the end of the day, of course, I hold no leverage over you. If you, internet stranger, want to use this styleguide in some other way not suggested above there is precious little I can do about it. Just remember how and why it works in its original setting if you find its performance underwhelming.

It is unlikely that a highly specific Git styleguide such as this will work for every conceivable type of software project and project maintenance model. An honest effort has been put into making the styleguide generally applicable, but even so a few project types will just not be able to honor every rule mentioned below. The vast majority of the rules will still be applicable, but you mave have to strike out a few.

Some projects conscientiously maintain multiple incompatible version of their software. A common example is a library that might continue developing and backporting features on their “1.x.x” release while the bulk of their development is focused on their new, incompatible “2.x.x” release.

In this case it is typically not possible to get by with just one long-lived branch as you genuinely need two differing branches that will never converge.

If you find yourself in this situation, you will have to make the following changes.

• Don't use There is one and only one long-lived branch: “main” at all.
• Change Changes to “main” happen through merge requests to list all your long-lived branches.

Assuming all of the rules in the styleguide are enforced, your team will reap a number of very tangible benefits.

There will be no quarrels about stylistic issues and aesthetic preferences; your usage of git is either wrong or right. If you have followed the guide to the letter, you will not be wasting time afterwards defending your idiosyncratic ways of using Git. If you fail to follow the guideline, you can be referred to the specific rule you're violating. Neither case has great potential to descend into a battle of egos and opinions. Your team members may be angry at the styleguide, but that is preferable to being angry at each other. They will go here, read, pull their hair, and eventually submit—just like with all the other styleguides in their software development life.

You know what to expect of your team members' actions. Will they branch off of your feature branches and expect that you will not rewrite the history of the branch you created? Will they merge directly into “main”? Will they merge or rebase? What sort of information can you expect from their commits? With a thorough styleguide you'll have an answer to all such questions and you'll be fully justified in expecting these rules to be honored.

When your entire team has submitted unquestioningly to the styleguide, your Git log will not be characterful and you will come to appreciate this. You will find your Git commit graphs have the same shape for all your projects, you will not look in vain for references to your issue tracking systems, and you will not suffer through reading walls of fixed lint error again. Instead your Git commits will form a uniform story of context, references, and reasoning. You may actually start using git log to answer questions about your projects as its output has become useful.

Some rules in the styleguide address the length of time that passes from branching out from “main” and until merging back into “main”. They attempt to minimize this duration, which means your team may have to change their task scoping to successfully follow the rules. The gold standard of this styleguide is to pick up a pending task, branch out from “main”, write your code with all that entails, put it up for review, and merge it back into “main” within a single workday.

Successfully concluding branches within a workday may seem trivial, impossible, or anything in between depending on your current development practices, but it is the natural conclusion to the quest to minimize the lifetime of branches. Achieving (or approaching, as reality permits) this goal puts a natural dampener on the size of changesets produced, which in turns greatly increases the likelihood that code review can be turned into a pleasant experience. I have not seen significant benefits for aiming significantly lower than one workday, but you should certainly not feel discouraged from solving many small tasks within a day. However, if you need a gold standard: aim to deliver one feature every day.

The decreased size of changesets will make code review faster, less taxing, more enjoyable, and will most likely increase the probability of catching each potential bug or misunderstanding as the reviewer will not be nearly as fatigued. The smaller changeset size also encourages code improvements as the reviewer and reviewee will both find the prospect of suggesting and implementing them less daunting given their relative ease. Finally, it is much more likely the reviewer will actually gain deep insight into the code, fostering a sense of shared ownership and knowledge sharing at a level that matters.

The increased frequency with which you'll review code, on the other hand, will make it a common occurrence. Contrasted with fewer and bigger code reviews, your team members will have many chances to learn to improve the process in this more comfortable setting, which should help the process converge on something efficient, enjoyable, and valuable. At least you've set the stage to encourage success.

It is invaluable to have clear, well-written, explicit, and opinionated knowledge to refer to when onboarding new colleagues. Just make sure the opinions match those of your team. Hopefully you will find this styleguide to check most of these marks. Simply tasking a new employee with reading such a guide immediately raises their knowledge about the shared values and practices of the team at almost no expense. As they improve it also serves as a reference guide when they invariably end up committing Git-stylistic crimes.

At the end of the day the purpose of a styleguide is to increase team happiness and spirit by removing common points of intra-team friction, creating a more cohesive team through shared values and mutual trust in everyone keeping some basic level of Git hygiene. This will not fix a struggling team, but it will likely improve a functioning team even further.

By and large, this styleguide is describing the branching strategy of GitHub Flow. That article covers most of the key concepts and much of the “how” and should serve as a general introduction if you need help on the more practical aspects of branching, but the rules that follow constitute the styleguide de jure.

The “main” branch contains the latest stable version of the project, it is always working and deployable, and it is deployed automatically to a non-productive environment when committed to. If someone breaks it, they have as top priority to bring “main” back in working order. However, breaking “main” should be rare since you should generally not get through code review and its automated tests with broken code.

There is no “dev”, “prod”, or similar branch with implied long-lived semantics. Any branch that is not “main” is readying itself to be merged into “main” as fast as possible, and should be considered both ephemeral and mutable. Branches that have been merged into “main” are deleted. If you leave old branches with non-merged changes in a repository, you are doing it wrong and you will likely be asked to either delete them or merge them.

If you feel there's a need to keep changes around, but you also feel unable to bring them into “main”, you should reflect on why that is. This should not be a common situation and there are many ways to remedy it. Maybe you need feature toggles. Maybe you need better planning. Maybe you need a mindset of shipping to production every day. This styleguide will not fix the underlying issue, but it is undoubtedly worth fixing.

No developers may commit directly to the “main” branch with the sole exception of initializing the repository on the “origin” server. The process for changing “main” is to create a merge request from your feature branch into “main”, which triggers a code review. This ensures there's always at least one second pair of eyes on changesets, it ensures CI has a change to reject failing builds, and it fosters a sense of shared ownership over changes.

Most modern Git hosts support features such as: preventing merges to “main” outside of merge requests, requiring approvals to merge, etc. You should make use of these to help enforce the rules. You should not except yourself from these rules.

It is essential to enforce that at least one other person takes the time to thoroughly review each and every change. There should be no exceptions. If you want a change to go to production, then it must be reviewed by your peers.

It could be your team size or structure permits or requires that more than one person reviews each changeset. That's perfectly fine, too, just set a minimum of 1.

When a developer creates a feature branch, they own the branch and they are free to treat it however they want. This includes rewriting history or deleting it outright. You should never assume that feature branches are stable, and branching off from a feature branch is both a bad idea, and done entirely at your own peril.

If for some reason you must base your own branch on top of an existing feature branch, the process of keeping in sync is your own headache, not the headache of the feature branch owner. You'll have to resolve this through oral agreements and rebasing, and ideally by not doing it at all.

It is not only possible, but expected and even encouraged, that feature branches will have their history overridden.

Seeing how you're diligently following the other rules in the stylesguide you will undoutedly have to make smaller or larger changes to your feature branch based on review feedback. Since feature branches are mutable you can safely amend the latest commit on your feature branch rather than creating a new commit for each piece of feedback. This will leave you with much less junk to clean up when it's time to merge.

Consider the task of cleaning up these two branches' commit logs.

  <lastest commit on main>
│   Add more logging
│   Improve main loop performance
│   Fix type in comment
│   Fix algorithm error
│   Reach code coverage goal
│   Refactor utility function
│   Fix README typo
│   <good commit message>
│╱
  <where you branched from main>

  <lastest commit on main>
│   <good, revised commit message>
│╱
  <where you branched from main>

The first situation is clearly more work to rewrite than the second, which doesn't need rewriting at all, but there's actually a more harmful danger lurking in the first example. If the review has been enjoying a lot of back and forth, changing many aspects of the code along the way, it's very likely that the resulting commit must be rewritten to a considerable degree to remain truthful to the final state of the changeset. Perhaps you've made some performance claims about the code that no longer hold, perhaps you've switched overall architecture entirely. There are many possible ways for the original commit message to grow outdated.

If many changes have been implemented based on feedback, pushing a new commit for each change, most people eventually grow fatigued in their writing efforts and end up producing genuinely terrible commits. This is understandable and is usually accompanied by a genuine desire to rewrite the branch history as one good commit when the tidal wave of feedback abates. However, there's a real risk that you may have a hard time remembering all the details that have changed and why.

If you make an honest effort to rewrite the commit message afresh with each batch of feedback you implement, you'll have the changes and context for them fresh in mind when amending the commit message. This doesn't completely remove the risk of forgetfulness, but it does reduce it. It also sets you up to simply hit “merge” when the merge request is accepted with no further writing efforts required.

Some people spend inordinate amounts of time crafting the perfect name for their feature branch. Some people like to prepend some carefully chosen string like “feature-” to their branch name. Some people dive into impressive meta-summarization efforts where they try to further summarize the imagined resulting commit subject line, which is of course already summarized. Some people develop complicated flows for naming branches depending on some combination of issue tracking tickets IDs and labels. The list goes on.

This is a complete waste of time when following this styleguide as no trace of the original branch name will remain after your changes make it into “main”. The branch will be deleted and the name of it will not figure in the commit on “main”. For the purpose of this styleguide, there is exactly one interesting part of naming feature branches that you need to get right: don't waste your time on it.

• Do you work with issue tracking tickets with IDs? Use that: “proj-123” makes for a great branch name. When working with tickets, naming your branches after a specific ticket may help you focus your efforts on a specific goal, which is great.
• Do you use post-it notes or make up your work as you go along? Just pick “feature” as branch name. Over and over. Nothing bad will come of it if you clean up both locally and remotely.
• Does your workplace enforce some perverse branch naming rules to no benefit when following this styleguide? Find some heuristic that lets you quickly spit out compliant branch names. As said, they don't matter and there will be no dirty evidence left of the branch or on “main” after you're done.

Committing in Git is a full topic in its own right. This section attempts to explain only a narrow subset relevant for the more stylistic properties. If you need help on how to commit there are better guides available.

A Git commit message consists of two parts: A subject line and an optional body. The subject line and the body are separated by two line breaks. The body, when present, contains one or more paragraphs separated by two line breaks. Here's an example of a Git commit message:

Support new OAuth2.1 flow

JIRA-123, ZENDESK-234

Since the OAuth server now supports OAuth 2.1 with PKCE it's time to
update the SPA. The flow still uses the Authorization Code Grant, but
now includes PCKE as required by OAuth 2.1.

It is quite mediocre in quality. We'll learn later how to improve upon this, but that is not the topic of this section. This commit message has a subject line:

Support new OAuth2.1 flow

and a body with two paragraphs:

JIRA-123, ZENDESK-234

Since the OAuth server now supports OAuth 2.1 with PKCE it's time to update the SPA. The flow still uses the Authorization Code Grant, but now includes PCKE as required by OAuth 2.1.

A short subject line means that tools will be able to render it in-line across their UI without truncation or unintended breaking of lines and boundaries. It also forces you to summarize.

You may find that this rule causes a lot of friction. If so, it's all the more important to reflect on the purpose of encouraging a summary. If you struggle to succinctly explain the changeset you may have fallen prey to one of the following bad habits.

• Including too many details in the subject will quickly send you over the limit. It's also unnecessary. You have all the space you need for details in the body.
• Solving multiple unrelated issues at once will lead to a messy subject line for the simple reason that the changeset is also messy. Consider solving one issue at the time if possible. If you absolutely must solve multiple issues in one commit, they'll most likely share the same motivating source which you can try to describe. E.g. increasing font size, darkening text color, and lightening background color might all be done to increase readability.
• You may be overthinking it. It's great that your commit will “Fix the concurrency issue that causes customer orders to occasionally be billed twice on Sundays”, but you could also make peace with how your commit will “Prevent double-billing bug”.

Writing is hard and summarizing is hard. Don't despair. We all grow better with practice.

A properly capitalized subject line or body can be directly copied from the commit message and into e-mails, release notes, or documentation as a title or sentence. Having to correct this for many commits is both annoying and more time-consuming than just getting it right when writing it in the first place.

There are two important aspects to this rule: subject lines have a verb as focal point, and the mood implies the direction of change. Let's motivate this with some examples:

Long line wrapping

Have we added or removed wrapping of long lines? Were we unhappy with the current long line wrapping, and thus got rid of it, or did we add it as a new feature? Clearly just using a composite noun as subject line leaves a lot of room for interpretation, which is a fertile breeding ground for misunderstandings. We could phrase it as a full sentence with a verb:

Wrapped long lines

The committing developer probably means that they wrapped long lines, and thus added line wrapping, but there's still the nagging possible interpretation that they have dealt with the problem of wrapped long lines, thus removing the wrapping. This, along with the slightly improved brevity, is the reason for preferring specifically the imperative mood:

Wrap long lines

This is a direct command and hence, in terms of the English moods, the strongest possible way to communicate that we want wrapping of long lines to happen, and hence that this commit introduces it. It minimizes the risk of misinterpretation at no appreciable cost. It also serves the (less important) purpose of always being shorter or as short as the other grammatical moods, and hence lends some small support in keeping with the length limitation set on subject lines. Finally, with all subject lines having the same grammatical mood, it enables a very simple way to summarize a merge request:

Accepting this Merge Request will:
– Support new OAuth2.1 flow
– Deprecate OAuth2.0 without PKCE
– Disable OAuth2.0 implicit grant

Having this consistency and absence of ambiguity is very handy when summarizing commits in many contexts.

This may seem purely stylistic, but there are two reasons to skip punctuation in subject lines: brevity and discouragement of multi-sentence subject lines. Not ending in a period is always shorter than ending in a period, and refusing to let subject lines end in periods highlights the wrong-doing of having multiple sentences per subject line:

Wrap long lines. Optimize fonts for readability

Hopefully, this looks clumsy to the observer. The solution is to summarize your changes in one sentence. Perhaps something like:

Improve article readability on mobile

and then flesh out the context and reason behind the changes in the commit body.

It is very tempting to write commit messages that describe what you changed. It is certainly easy, too, but it is not very useful. Consider this commit message:

Update oauth21.js

Granted, it leaves us in little doubt as to which files changed, but we don't actually need this information in the commit message.

There's a much more reliable and comfortable way to figure out which files changed in a given commit—just use Git:

> git log --stat
commit abcabcabcabcabcabcabcabcabcabcabcabcabca
Author: Antediluvian Developer <noreply@antediluvian.dev>
Date:   Mon Jan 10 10:20:30 2000 +0100

    Support new OAuth2.1 flow

    JIRA-123, ZENDESK-234

    Since the OAuth server now supports OAuth 2.1 with PKCE it's time to
    update the SPA. The flow still uses the Authorization Code Grant, but
    now includes PCKE as required by OAuth 2.1.

 lib/oauth21.js    |   3 +++
 1 file changed, 3 insertions(+)

This explains exactly which files changed and how much. But what about describing which changes were made to the contents of the files? Let's imagine the commit message subject line is changed to:

Call new PKCE function

This turns out to be equally unenlightening. Git will happily include the exact details of how the contents of those lines changes when asked to:

> git log --patch
commit abcabcabcabcabcabcabcabcabcabcabcabcabca
Author: Antediluvian Developer <noreply@antediluvian.dev>
Date:   Mon Jan 10 10:20:30 2000 +0100

    Support new OAuth2.1 flow

    JIRA-123, ZENDESK-234

    Since the OAuth server now supports OAuth 2.1 with PKCE it's time to
    update the SPA. The flow still uses the Authorization Code Grant, but
    now includes PCKE as required by OAuth 2.1.

diff --git a/lib/oauth21.js b/lib/oauth21.js
index 1231231..2342342 100644
--- a/lib/oauth21.js
+++ b/lib/oauth21.js
…
     // Really important code stuff here.
     someOldCode();

+    // The new code here.
+    ensurePkceStuff();
+

Not only is it irrelevant to mention which files you changed or how the lines were transformed, it is subtly harming the usefulness of your Git commit log due to a number of compounding factors:

• It's easy to do because the information is readily at hand (Git even summarizes it for you when you write you commit message). This makes it a very alluring choice if you're too lazy to spend time on writing your commit message properly. It's an escape hatch that transforms the cognitive effort of writing into the mechanical effort of repeating what's already listed by Git. It lets you off the hook easily, and it may even feel like you've done well, but you do so at the cost of polluting the commit log with useless junk.
• If you spent your precious time writing words about which lines changed and how, you didn't spend it writing what actually matters. You wasted your precious time and mental capacity and produced no actual value.
• You'll occasionally get the facts of which files changed and how wrong, either by mistyping, forgetting, or by amending your changes without correcting your commit message. This makes it a strictly worse source of truth for changes than Git's own log, which is not capable of any of those transgressions.

So what should you write about, then? All the harder stuff.

Git doesn't know why you changed the code. Your team members won't always know why you changed the code. YOU won't remember why you changed the code six months from now. But some day some poor soul will be trying to figure it out, and they'll have a much better time if it's readily available in the Git log.

Let's walk through an example of progressive commit enhancement.

This is terrible:

Update style.css

Changed body font family and size.

All we learned is a subset of what's available from git log --patch. The changeset has our back in relaying how you changed “15” to “18” and “Times New Roman” to “Palatino Nova”. You don't need to repeat these facts.

This is an improvement:

Make body text more readable

Some customers had a hard time reading the body text so we increased
the size and picked a better font family.

Now we know that customer feedback inspired the change and that readability was a problem. But I'm not sure I understand why the font family had to change.

This is passable:

Make body text more readable

ZENDESK-123

Some customers had a hard time reading the body text so we increased
the font size by about 20%.

It also turned out the company branding guide lines dictate Palatino
Nova as the font family for body text, so we changed that.

Great. Now there's a reference to the ticket raised by the customer so we can read up on it if we need to. It also seems the font family change was an unrelated change that just got lumped in. That's fine, and we gained a glimpse into why the events unfolded as they did.

However, we can do even better still.

This is what I like to find in my Git log:

Make body text more readable on mobile

ZENDESK-123, JIRA-234

Some customers had a hard time reading the body text so we looked into
the best way to present the text at all three media query break points.

After talking to Miriam from UX it turned out we were not following the
corporate branding guide lines in a couple of ways:

*   We should always use Palatino Nova for body text.
*   The breakpoint (max-width: 600px) should use 18px instead of 15px
    font size.

The guide lines turned out to be very human-centric and not nearly as
draconian as imagined.

Miriam also helpfully suggested we increase the contrast ratio of body
text to background at least 7.5:1, which has been fleshed out in the
JIRA story referenced above.

The commit message includes a good deal of helpful context:

• Miriam from UX is a great person to contact in the future about similar matters.
• There's a really good corporate styleguide we should strive to follow.
• A follow-up story was created to further increase readability.
• Different screen resolutions and form factors seem to have been taken into account.

Depending on your current level of commit message ambition the last commit message may seem completely bog standard, a Dostoyevsky-sized novel so voluminous that you could not see it to completion in this life, or somewhere in between. I'm telling you it's the former. It's a completely reasonable commit message that took me a couple of minutes to write and took no exceptional skills beyond a genuine interest in the mental well-being of my colleagues and future me.

Get into the habit of providing the best context you can in your commit messages. You, future you, and your colleagues will all be better for it.

I appreciate that writing is hard, and that is exactly why you have such ample opportunity to make a real difference if you put some effort into it.

The various Git hosts differ slightly in terminology, but all the major ones support the concept of a request to merge one branch into another accompanied by a platform for comments, CI pipelines, and approvals. This will be refered to as a “merge request” for simplicity, but you can substitute whichever terminology your Git host of choice employs.

The point of a merge request is the ability to have a discussion around proposed code changes to ensure a high quality and a sense of shared ownership of the code. Team members will pitch in with questions, remarks, and suggestions, and after some back and forth everyone should agree that the code is in as good a state as circumstances allow. At this point at least two people should be familiar with the purpose, implementation, and possible issues regarding the code change.

All code changes come with both risk and cost—some negligible in amplitude, some very much not. A healthy default stance on code changes, then, is “no thanks”. This is not a problem in itself, but it does put the author at a disadvantage and the reviewer under pressure to scrutinize, at times perhaps even unduly so.

Seen in this light, the merge request can become a vessel for all sorts of social friction, power struggles, and toxicity. One party has labored hard to produce something, which they now fight tooth and nail to sell. The other party has been selected to evaluate the merit of the work, and they are under pressure to formally object to it or endorse it.

Don't underestimate the power imbalance and potential for psychological abuse. You need to develop a strong culture and clear rules to alleviate the inherent difficulties with which this domain is fraught.

The rules that follow make an honest attempt to address both technical and psychological aspects of code review.

Imagine two scenarios:

1. You've been asked to review a code change, but you have no idea what it's about or why the code needed changing in the first place. Both the business case and the end user's perspective are unknown to you.
2. You've been asked to review a code change, and you've been given a quick two-paragraph fly-in to what changed and why. You understand both the business case and the end user's perspective on the matter.

Let's not beat around the bush: the scenarios are set up for failure and success, respectively. It's that simple.

A reviewer who is deprived of context will need to initiate their review with a throng of questions the the effect of: “what is the purpose of this change?” and “what is the company context surrounding this change?”. The reviewer is bewildered and the author is on the defense. Neither is off to a good start.

The good news is that if you follow this styleguide to the letter—particularly the parts about writing a good commit message—the subject line will serve as merge request title and the commit message body as merge request description. If you've written them well, they should be perfectly suited to alleviate the problem. This is, in fact, what many Git hosts use as the default title and description for their merge requests.

Having read the title and description, your team members should have the context needed to discuss the changes introduced. This removes one possible point of friction from the process.

As a reviewer, questions are usually the best way to give feedback for a number of reasons. When phrased properly, they feel less judgemental and more open to discussion than their counter parts, the statements. Here are some examples with commentary:

• This should be a constant. vs. Can this be made a constant?

  The former comes off as judgemental and without much recourse for the author, who is factually wrong about the choice of not using a constant. The latter comes off as curious about a possible improvement, and with the distinct possibility of the question being the product of the reviewer's lack of insight into the changeset.

• This code has a bug and throws an error when user is undefined. vs. How does this part deal with the case where user is undefined?

  In the former there is no room for the author to have considered this case and for the reviewer to be wrong. In the latter there is.

• This algorithm scales poorly and will be a performance bottleneck. vs. How does this section perform if the array grows very large?

  They mean the same thing, but one makes the author feel like they've made a mistake, and the other makes them feel they're being asked for their opinion on a tricky technical issue. The details really do make all the difference.

Does it feel awkward at first? Maybe. Does it completely remove all social friction from merge requests? Probably not. But it really does help, and you will quickly get used to it.

As a reviewer, never make any mention of the author; their personality, beliefs, or traits; or their past work in a merge request. Instead focus single-mindedly on discussing the code changes.

A merge request is intended to review code, not its author. Even, or perhaps especially, if you've made mistakes in the past, you're entitled to a clean slate in each review to keep the work environment friendly and to encourage professional growth.

• Just like last week you keep forgetting to validate the input. You should use the ‘Joi’ library for validation. vs. Would this module benefit from using the ‘Joi’ library to validate input?

  There is just no need to bring up last week's mistakes, any tendency in the author to forget this or that, or indeed to mention them at all. I bet they remember last week's mistake quite keenly without any reminder.

• You have chosen that algorithm really cleverly. 👍 You're so good at this. vs. That is a really clever solution. 👍

  Admittedly, this is lower on the list of offenses than the previous, but hear me out. You are discussing the code changes and their impact on the code base. You are not congratulating each other on your great mental acuity and coding exploits. Much like you don't want to chastise your team members for their failings, you don't want to veer too far towards veneration either.

  This is particularly bad if you keep glorifying a specific team member while never acknowledging another. You're certainly allowed to prefer the code of one of them to the other, but it's much more helpful to attempt to raise the code quality of the lower perceived performer than to profusely congratulate the higher perceived performer on their excellence.

An ideal merge request is the team against the code. Don't scold anyone. Don't praise anyone. Just keep the talk about the code, follow the rules here to make it actionable and positive, and you'll eventually build this sense of a unified team that stands with each other against bad code. You have plenty of opportunies outside of code review to congratulate each other on your merits.

As a reviewer, it's your responsibility to “get it”. The author likely already knows the point of the merge request. They won't feel inclined to drive your understanding of the code change, and neither should they. It's up to YOU to keep asking questions until you understand the larger context of the change, why it's needed, why it manifests this way in code, and what the code actually does.

If your team is following the rules above, you should be well on your way after reading the merge request description, and you should be acquainted with asking low-friction questions that are positive and actionable.

Ask plenty of questions. There is no reward for falsely pretending to understand every code change, except perhaps a growing ignorance of your team's body of work. There is no shame in asking questions. You're not assigned as reviewer to test your virtue as some sort of expert auditor; you're assigned as reviewer to make sure at least one person beyond the author fully understands the change. If you happen to be a domain expert—great, that's an added bonus. However, the primary purpose is to gain understanding.

You'll likely find that this sort of genuine inquiry nibs a lot of issues in the bud.

As a reviewer, try to be as specific as possible. Why is this written like that? takes more effort to digest than How come this breaks the code styleguide?, which in turn takes more effort to digest than Would breaking this method chaining onto multiple lines increase readability?. It takes about the same amount of time to write, however, and it's less likely you'll need to explain what you meant.

Specifics are also less likely to be misintepreted as a general comment about your team member's traits, habit, or personality. Consider the following.

• Why?—very unclear scope with high risk of being taken as a general and personal comment about any and all worries rummaging around the author's head.
• Why not follow the styleguide?—at least gives a general direction for the complaint, but still sounds like it could apply to large swaths of code or even previous work.
• Why not apply our eslint styleguide §5.8 and split it into multiple lines?—much better, but “why” questions aren't the best. See the next section.
• Would the readability improve if we applied eslint styleguide §5.8 and split it into multiple lines?—much better. Suggests a specific positive outcome, opens up room for negotiation, language suggests joint ownership over the change.

There's nothing inherently wrong with the word “why”, but it's very easy to take an unfriendly comment like

Your code is terrible.

and rephrase it directly as

Why is your code terrible?

in a mostly mechanical fashion. It's an easy habit to fall into, and while it certainly doesn't HAVE to result in hurtful phrasings, it has a higher risk of doing so than certain other means of communication. Some of the other rules in this section (like suggest an alternative provide safer approaches to voicing your concern as a reviewer.

“Why” also has a tendency to appear confrontational or inquisitorial when read by a merge request author. It's just one of those times where reading a phrase comes off harsher than writing a phrase. There is often an invisible, implied “you, the author” somewhere in the question when it starts with “why”.

Here are some examples of “why”s and how the alternatives appear much gentler.

• Why not check for null?

  How will this code deal with null?

• Why wasn't the “luxon” library used?

  Would this code be simpler if we introduced the “luxon” library?

• Why add this endpoint?

  Can I get a quick introduction to this endpoint? I'm completely unfamiliar with it.

Every time you start a review comment with “why”, read it again just to be sure it comes off in the right tone. If not, rewrite it in some other form. This is particularly important if it's a very short comment as brevity seems to aggravate the issue.

No one likes being told they've done wrong, yet it is often at the essence of reviewing code changes. It's typically more palatable to be wrong if you're being offered a specific alternative and given agency in whether to use it. Consider an example.

• This code is so complex that it's basically unreadable.

  This is not pleasant, not actionable, and doesn't really teach a better way. This is a terrible comment.

• Could we reduce the code complexity here? It is quite hard to read.

  This feels less condescending, but it's not really any more actionable, and still doesn't teach a better way. If the author knew how to solve the problem with code of lower complexity they probably would have the first time around.

• Could this be made less complex using ‘Array.prototype.map’ and ‘Array.prototype.reduce’?

  This feels much less condescending, and it is actionable. However, since the author didn't use the suggestion in the first place they may not be familiar with the technique. This is a decent comment, but it can be made even better.

• Would this be more readable using some of the techniques in this other commit from our git log (insert link)? Perhaps something in the vein of this code snippet (insert snippet)?

  This doesn't feel condescending, it is very actionable, and there's a specific suggestion and a reference to an example in the wild if the author wants to study the technique further. The language is about readability, which is a positive trait; not complexity , which is typically a negative trait.

Some developers react to the responsibility of having to review code by conjuring up various excuses to not do so. Typically this weaseling out will be accompanied by some hand-waving about “trusting the author's ability”, “not wanting to waste time”, “relying on CI and tests to catch bugs” and similar statements that completely miss the point of code review.

This malpractice, commonly called “rubber-stamping”, deprives the team of all the essential parts of code review: knowledge sharing, a sense of shared ownership, and code quality assurance. You can not let this happen.

If you have the impression that reviewers are just quickly skimming the merge request and hitting “approve” without any examination, reflection, and questions about the bigger picture of the change, you need to go back and work on establishing code review as a pillar of your team culture. You can not succeed under these conditions.

Talk to the culprits, have a workshop, involve your boss if push comes to shove. If left to fester this will completely void all of the benefits of code review.

When working on feature branches and eventually merging them into “main”, you have a few footguns at your disposal that can greatly reduce the usefulness of the resulting Git log. This section is dedicated to the ways Git branches should and should not be merged and rebased into and onto each other.

This section assumes more knowledge about Git than those before it. At the very least you should understand what commits are, how they may be viewed as a graph, and what merge commits are on a superficial level.

Even though your feature branches are surely short-lived in accordance with this styleguide, you will eventually encounter a situation where “main” moved under your feet as you were writing your code. This is not a problem in and of itself, but it leaves you with the task of reconciling this difference between the “main” you branched from and the “main” you now want to combine with.

The general gist of this section is to always rebase your feature branch onto “main” and never use merge commits.

After developing a new feature you might be left with two branches, “main” and “feature” (an example branch name we'll use throughout), with a diverging history.

        A   C   E
main   ┄ ─── ─── ┄
         ╲
feature    ─── ──┄
          B   D

In this example your feature branch and “main” have the most recent common ancester commit “A”, but otherwise diverge. “main” has commits “C” and “E”, while the feature branch has commits “B” and “D”.

You might be tempted to merge “feature” into “main”:

        A   C   E   F
main   ┄ ─── ─── ─── ┄
         ╲         ╱
feature    ─── ───╴
          B   D

Here “F” is a merge commit with both “main” and “feature” as parents.

Don't use merge commits. If you can accept that, you can skip the remainder of this rule. What follows is one long harangue against merge commits.

Merge commits preserve the "lump" on your git graph created by the feature branch forevermore and for what is most often absolutely no reason at all. Some people like having a merge commit as they imagine they'll surely need it in order to easily revert the changes if regressions are found. It is factually correct that reverting a merge commit conveniently undoes its changes, but the argument doesn't hold up to further scrutiny in the general case and is plain wrong in the context of this styleguide. Consider the following:

• Since you only have one commit per feature branch as per this styleguide, reverting that commit is no harder than reverting a merge commit. Even if you had a handful of commits, reverting them is quite trivial.

• The command “git revert” has some gnarly pitfalls that often manage to trick newcomers, which is why this styleguide prohibits its use entirely. We'll discuss why at some length later. This makes the argument for having one commit to revert much less convincing.

• You should be striving to roll forwards, not roll back. If your “main” branch has a serious defect, you should fix that rather than revert recent changes when possible. If you're practicing small commits, this should be easy in the general case.

• The artifact deployed as your production environment (be it a web service, library, CLI tool, or something else entirely) and the head of “main” should be decoupled in a sane development and deployment setup. If they are, you don't need to revert “main” in a panic if a defect is found—you need to roll back your production environment in a panic instead.

• The combination of an additional commit and the "lump" doesn't exactly embellish your git log.

This is how the git log looks after merging.

git log --all --graph --oneline
*   5888408 (HEAD -> main) F
|\
| * eb4c9c9 (feature) D
| * 5ef9ee0 B
* | 3408edd E
* | 1ba3307 C
|/
* c809827 A

You might be arguing that this is somehow “the truth of what happened” and that these facts must be recorded for posterity. Much like the most generally useful format for getting a good, quick sense of what happened in human history employs curation, filtering, and careful presentation over heaping stacks of disorganized notes, recordings, and statements, so should your git log employ these tools. You're not trying to preserve the exact order and fashion in which every commit actually happened—you're trying to make it as useful as possible for you, your colleagues, and future you.

If you remain unconvinced then consider this final argument. You surely have CI running all sorts of tests and verifications against your feature branches. If you follow the merging practices described above, then your CI will be testing different code than that resulting from your merge. If your CI tests commit “D”, then it is really testing the union of (“A”, “B”, and “D”). What will result from the merge is the union of (“A”, “B”, “C”, “D”, and “E”). In other words, you're not testing what will result from the merge and you might miss integration issues.

Imagine, again, that you're working on the branch “feature” with the following commit graph:

        A   C   E
main   ┄ ─── ─── ┄
         ╲
feature    ─── ──┄
          B   D

Some people prefer to locally reconcile their differences with “main” by merging “main” into “feature” like so.

        A   C   E
main   ┄ ─── ─── ──┄
         ╲       ╲
feature    ─── ─── ┄
          B   D   F

Here “F” is a merge commit with both “main” and “feature” as parents.

This allows for any conflicts or issues to be resolved in the feature branch and also for CI to run against what will actually result from the merge. Of course to get the changes in “feature” into “main” one additional merge is needed:

        A   C   E   G
main   ┄ ─── ─── ─── ─┄
         ╲       ╲ ╱
feature    ─── ─── 
          B   D   F

Here both “F” and “G” are merge commits with both “main” and “feature” as parents.

Don't use merge commits. If you need a refresher on why that is, it is fleshed out in more detail in don't just merge into master.

Compared to the previous merge model this is better in one regard and worse in others. Specifically:

• We're now testing the right code when CI tests “F”, which is good.

• We've doubled the number of indecorous, anemic merge commits that do not contribute anything much to our git log, which is bad.

• We now have two merge commits per feature, which makes it even more likely that newcomers to Git will manage to use “git revert” in ways that surprise them and their team mates.

The disservice done to the git log is greatly exacerbated.

git log --all --graph --oneline
*   f2478d5 (HEAD -> main) G
|\
| *   14bcf4e (feature) F
| |\
| |/
|/|
* | 3408edd E
* | 1ba3307 C
| * eb4c9c9 D
| * 5ef9ee0 B
|/
* c809827 A

Is this an improvement over the previous model? I don't want to spend much time comparing them as luckily there's a model with all of the benefits and none of the drawbacks. Read on.

Imagine, again, that you're working on the branch “feature” with the following commit graph:

        A   C   E
main   ┄ ─── ─── ┄
         ╲
feature    ─── ──┄
          B   D

The trick is to require that “feature” is able to fast-forward merge into “main”. That is, require that “feature” is a descendant of “main” or alternatively that all commits on “feature” have head of “main” as descendant.

This is how the graph must look prior to merging:

        A   C   E
main   ┄ ─── ─── ──────┄
                 ╲
feature            ─── ┄
                  B   D

The resulting git log looks equally simple.

git log --all --graph --oneline
* 703551b (HEAD -> feature) D
* 882dba1 B
* 3126421 (main) E
* d4555a6 C
* e4d1cc1 A

At this stage you can fast-forward merge “feature” into “main” (or reset “main” to commit “D”), which will merely move the head of “main” to point to D.

        A   C   E   B   D
main   ┄ ─── ─── ─── ─── ┄

You may be able to guess what the resulting git log looks like.

git log --all --graph --oneline
* 703551b (HEAD -> main) D
* 882dba1 B
* 3126421 E
* d4555a6 C
* e4d1cc1 A

Compared to the previous solutions we reap all the benefits with none of the drawbacks:

• Our CI will test “D”. After merging, the head of “main” will become precisely “D”, ensuring the right code is tested.

• All commits left in the log are hand-crafted and serve specific feature-related purposes.

• The log is perfectly linear with no lumps.

• It is still completely possible to undo the changes introduced by one commit or some commits. Merge requests are not an enabling feature for this.

If you already feel thoroughly convinced that merge commits are to be avoided you can safely skip this section. It does what it says on the tin and merely picks up disputing merge commits where the previous sections left off.

One particularly nefarious pitfall that often manages to trick Git users is the combination of merge commits and reverting commits. Imagine the following scenario.

1. You have branched off from “main” to develop a new feature on the branch “feature”.
2. You merge your changes into “main” using a merge commit.
3. An angry throng of end users, product owners, and operations people call you up, suggesting in unambiguous language that a regression has been identified.
4. You recognize that a dependency needs updating before your change can go live and you revert your merge commit for now.
5. The dependency is fixed and you want to reintroduce your changes. You attempt to merge “feature” into “main”, but “main” insists that…

6. # git checkout main
   Switched to branch 'main'
   # git merge feature
   Already up to date.

7. You hate Git now.

I can't say whether this behavior is what you expected or not, but I can say with some certainty it will not be what every one of your current and future team members will expect. It is one of the classic pitfalls of Git that continues to surprise and anger users, and as it turns out it is completely avoidable.

Don't use merge commits.

The rules above have all focused on getting the concepts, motivation, and general approach right. This section lists some useful and specific Git commands to implement the workflow that results from following the rules.

The commands listed below are certainly not the only way to do so, but they should cover most day to day needs.

Throughout this section I assume you've picked the name “origin” for your remote. If you have not, you'll have to substitute your chosen name.

Let's recap the typical workflow that results from the rules.

git checkout main

This switches to the “main” branch to ensure you're not branching from a previous feature branch.

git fetch origin main

This fetches any changes to “main” from “origin” to ensure you're not branching from an old state of “main”.

git reset --hard origin/master

This changes your local HEAD of “main” to be the HEAD of origin's “main”. You're left very sure you're branching off from what's on origin with no local additions you may have introduced by accident.

git checkout -b feature

This creates a new branch called “feature” and switches to it, leaving you on the new branch.

This is left as en exercise for the reader.

git status

Take stock of the state of your local files. Make sure all the files you expected appear here, and that no unexpected files appear.

Do you have new, untracked changes? If so, you should declare your intent to add them. Otherwise skip this step.

git add --intent-to-add <untracked files>

With all your intended changes now tracked, add them interactively to be faced with each change you've made.

git add --patch

This will present you with each chunk of changed code and have you decide—by entering “y” or “n” interactively—whether to add each chunk to what will be committed.

git commit

This will open up your editor and let you write a good commit message. If an undesirable editor opens up, just close it without entering any text, make sure to configure git to use your preferred editor, and try again. This is how you might configure your default git editor to be VS Code (picked due to popularity) across all of your repositories.

git config --global core.editor "code --wait"

This will use the same upstream branch name as local branch name, which is probably what you want to not unduly confuse yourself. If your intended branch name already exists on “origin” you may have to pick a different one.

git push --set-upstream origin HEAD

Most git servers will respond with a chunk of text in your terminal that includes a link to open a merge request from your feature branch into the default branch. It's fine to follow this link if it's available to you.

Usually you'll have received a link for opening a merge request directly in your terminal from carrying out the previous step. If not, you might have some CLI tooling available to do so, and if all else fails you should be able to go to your git server host and create one.

This step is inherently dependent on your specific git host. If it's unclear what to do, your team members should be able to help you out.

As review feedback comes in, it might be necessary to amend your code to implement suggestions or fix issues. After dealing with each batch of feedback, you can just amend your commit to incorporate the new changes.

If you have new untracked files, proclaim your intent to add them.

git add --intent-to-add <untracked files>

Add your changes one by one, looking through each interactively before choosing to add it or not.

git add --patch

Commit your changes to rewriting your previous commit, keeping only one commit on the feature branch.

git commit --amend

Make sure to check whether your commit message is still accurate and amend that as well if necessary.

git push --force-with-lease

To override the remote, you will need to “force” your push as git is naturally and sensibly hesitant to rewrite history. Be sure to use specifically “--force-with-lease” as this will ensure you only override the remote history if the current value on the remote server is the same as what you have locally in your remote-tracking branch. In other words, if anyone changed the remote since you last updated your remote-tracking branch, this operation will fail. If everyone is following the styleguide that should not happen, but if they did by mistake you'll probably want the option to sort out what happened before just deleting it from history.

Most likely this will happen through the web interface of your git host. Typically there will be a big button labeled “Merge” that will do what it says on the tin.

If at all possible, make sure to manage merges into “main” exclusively from your web interface and try to remove every team member's permission to manually push to “main” at all on “origin”

If you're presented with the option of a merge commit or a fast-forward merge, be sure to pick fast-forward merge.

However, if your git host for some reason does not support merging directly from the merge request web interface (unlikely in most setups I've seen) you can do so manually.

git checkout main

This ensures you've switched to the branch you intend to merge into.

git fetch origin main

This ensures you've fetched the lastest changes from “origin”.

git merge --ff-only origin/main

This merges any changes from origin into your local “main”, but only if the changes can be fast-forward merged. If not, you'll need to figure out why your history disagrees with that of origin.

git merge --ff-only <feature branch name>

Merge the feature branch into “main”, but only if it can be fast-forwarded, which is a hard requirement in this styleguide.

git push origin main

Push “main”, which now includes the commit(s) of the feature branch, to the remote server.