Git Etiquette: Submitting a Pull Request

This post was originally written as an internal article on Cheesecake Labs’ wiki. It merges (no pun intended) pull request practices from many different cultures and backgrounds into a definitive workflow. We will discuss why you should have a guide for managing your PR lifecycle, as well as tips on how to accomplish that.

Before opening a Pull Request

Double check your code! git diff is an excellent tool for that and it’s just a few keystrokes away! Try git diff master.. (where master is the branch you’ll target your PR with).

Think before using your CI service as a test runner as it might delay other people’s work, increase costs or just annoy people with a bunch of failure emails — if that’s the case, run them locally!

Filling up your PR submission

Ok, now it’s time to fill some web forms: insert a concise title with strong keywords, it will help people reference your PR later.

Now think with yourself: how would I review this PR? Did I commit by files, modules or by context? Is it easier to review commit-by-commit or the diff as a whole? Use the description field to let your reviewers know! Don’t assign people yet!

Before assigning people

Remember: you are about to assign a task to someone, they will eventually stop what they are doing to devote some attention to your code.

It is a good idea to take another look at your diff — yes, again — because origin’s target branch might be different from what you’d expect, it’s better to catch these issues sooner than later. Also, wait for tests and other checks (if any) to pass.

You should also understand that GitHub has the concept of assignee and reviewer. To my understanding, a reviewer is anyone whose responsibility is:

????????????????????????????????????????????????????????

review your pull request.

On the other hand, assignees are responsible for merging the PR.

So, if you want to merge the PR youself, only assign yourself! If you think it should be merged ASAP, you probably should also assign every reviewer, so they can feel free to hit the green button once everything is OK.

This practice is important to prevent premature/accidental merges. There are many scenarios where the PR code is ✅ but it still depends on some external factor. An example is a continuous delivery system that automatically deploys the master branch: sometimes you want to delay the deployment by a few hours, a day, etc. Having that rule in practice gives you peace of mind by knowing that you are the one controlling when the PR will be merged.

Something is broken ❌

Fix that broken piece! This is a good moment to use some advanced git skills like --amend, --fixup or rebase -i. Don’t feel comfortable using these commands? No problem! Do a regular commit fixing the issue and push it to origin.

All good now?

It’s time to assign people and wait for some feedback!

From now on, people are aware of your branch and might even have pulled it to their local machines, so it’s a bad idea to amend commits or fixups because it might cause undesired results.

Someone requested code changes

This is great! Someone devoted some attention to your code and kindly left you some comments about it. This might be a good moment to learn something new — or maybe it’s just a missing comma.

It’s time to get your hands dirty again! Fix the issues and preferably commit them individually or grouped by context —  avoid doing general commits like fix all requested changes— someday you may need to traceback some code change and it might be missing an useful message.

Leaving the office but haven’t finished fixing it yet? You may push the commits to origin without worries! Your reviewers shouldn’t spend time looking into your PR again until you explicitly say so.

I fixed everything!

After pushing all fix commits to origin, give it a few minutes to run the tests again. Did they pass? Take another quick look at the PR. All good ✅?

Let your reviewers know! Add a comment to the PR mentioning the reviewer(s), a simple @username ping! will do it!

Good job, now relax ????????

Wait for approval and ???? it to the world.

Bonus tips ⛱

Amending a commit is quite simple: Let’s say you forgot to add some stuff in your latest commit, all you have to do is git add [files] then git commit --amend.

Fixups commits are a bit more complex: if you want to add some changes to a previous commit you have to git add [files] then git commit --fixup [commithash_to_amend]. That will generate a fixup! commit message referencing the commit you wanted to amend with. After that you just need to do an interactive rebase with git rebase -i --autosquash [basebranch]and it’s done!

Check out this awesome post my coworker Bernardo did about Code Reviews.

Did I miss something?

Any other practices you recommend? Think this is too much pragmatism? Leave a comment below, let’s discuss it!