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
Double check your code!<\/strong> 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 Ok, now it\u2019s time to fill some web forms: insert a concise title with strong keywords, it will help people reference your PR later.<\/p>\n 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\u2019t assign people yet!<\/strong><\/p>\n 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.<\/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:<\/p>\n ????????????????????????????????????????????????????????<\/p>\n 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 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.<\/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 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\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>\nFilling up your PR submission<\/h2>\n
Before assigning people<\/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
Someone requested code changes<\/h2>\n