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.<\/p>\n
<\/p>\n
Before opening a pull request, double check your code and run the tests locally.<\/strong> Reviewing your own diff first catches problems before you take up a reviewer\u2019s time, and running the tests on your machine avoids delaying other people\u2019s work, increasing costs, or sending a bunch of failure emails. Think before using your CI service as a test runner as it might delay other people\u2019s work, increase costs or just annoy people with a bunch of failure emails\u200a\u2014\u200aif that\u2019s the case, run them locally!<\/p>\n Filling out a pull request means giving reviewers what they need before they start: a concise title built from strong keywords, so people can reference your PR later, and a description that explains how you would review the change yourself. Do not assign anyone until that is done.<\/p>\n Now think to 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\u2019t assign people yet!<\/strong><\/p>\n Before assigning anyone, take one more look at your diff and wait for the tests and other checks to pass. Remember that you are about to assign a task to someone: they will eventually stop what they are doing to devote some attention to your code.<\/p>\n It is a good idea to take another look at your diff\u200a\u2014\u200ayes, again\u200a\u2014\u200abecause origin\u2019s target branch might be different from what you\u2019d expect, it’s better to catch these issues sooner than later. Also, wait for tests and other checks (if any) to pass.<\/p>\n You should also understand that GitHub has the concept of assignee<\/em> and reviewer<\/em>. To my understanding, a reviewer<\/em> is anyone whose responsibility is to review<\/strong> your pull request.<\/p>\n On the other hand, assignees<\/em> are responsible for merging<\/strong> the PR.<\/p>\n So, if you want to merge the PR yourself, 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.<\/p>\n This practice is important to prevent premature\/accidental merges. There are many scenarios where the PR code is \u2705 but it still depends on some external factor. An example is a continuous delivery system that automatically deploys the Fix that broken piece! This is a good moment to use some advanced git skills like It\u2019s time to assign people and wait for some feedback!<\/p>\n From now on, people are aware of your branch and might even have pulled it to their local machines, so it\u2019s a bad idea to amend commits or fixups because it might cause undesired results.<\/p>\n When someone requests changes, fix the issues and push the fixes, then tell your reviewers when you are done. This is great news, not a setback: 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\u200a\u2014\u200aor maybe it\u2019s just a missing comma.<\/p>\ngit diff<\/code> is an excellent tool for that and it\u2019s just a few keystrokes away! Try git diff master..<\/code> (where master is the branch you’ll target your PR with).<\/p>\nHow should you fill out a pull request?<\/h2>\n
What should you do before assigning people to your PR?<\/h2>\n
master<\/code> 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.<\/p>\nSomething is broken \u274c<\/h2>\n
--amend<\/code>, --fixup<\/code> or rebase -i<\/code>. Don’t feel comfortable using these commands? No problem! Do a regular commit fixing the issue and push it to origin.<\/p>\nAll good now?<\/h2>\n
What should you do when someone requests changes on your PR?<\/h2>\n