Here, we start zoomed out at history, and zoom into the level of individual commits, looking at what we think are good and bad examples of each.
We care about history because it helps us understand where code came from and how it evolved. Keeping a clean history makes these tasks (sometimes much) easier. Git comes with tools like git-bisect that can only usefully be used with linearizable histories where the tests pass on every commit. In real-life development is nonlinear; that's why we have branches; but at the end of the day we'd like to create the appearence that development was done linearly, without losing the benefits of a distributed workflow (or forgetting how the work was actually accomplished).
We can probably all agree that we don't want our history to look like this:
And (arguably) ideally, we'd like our history to look like this:
feat1 o-o-o-o-o-o PR
/ |
master - - o - - - - - o - - - - - o - -
\ |
feat2 o-o-o-o-o-o PR
Note that despite the apparent branching and preservation of the context of each commit, master is actually a linear history of commits that can be easily bisected and understood. Most of the rest of this document is about how and why we do this.
Pull requests are one of the most important ways we communicate about, share context on, and get feedback on code.
A good pull request:
- is made as soon as a meaningful, mergeable chunk of work can be completed, to minimize the possibility of conflicts with other team members
- accomplishes a single product or engineering goal (or fraction thereof)
- is composed of a sequence of good commits (see next section), with the tests passing after each commit, and no merge commits if possible
- once you introduce merge commits, you can no longer rebase, and you will introduce a permanent nonlinearity into master.
- every commit should ideally pass the tests. later we'll explain how to reconcile this with 'commit early, commit often'.
- comes with a descriptive message explaining:
- what product or engineering goal it accomplishes (with links to necessary context)
- anything that might be unclear or unusual about the code (although this should probably be documented in the code as well)
- a description of how the code was tested (hopefully locally and/or on dogfood), with exposition of possible things that could go wrong that you tested.
- extra description if a change is irreversable (i.e. produces backwards-incompatible data) or introduces deploy dependencies
- is of manageable size, so it can be meaningfully reviewed quickly
- (except when necessary, e.g. for large refactors -- which should still be broken down and described in the commit message as much as possible)
Reviewing PRs should be a high priority for your team members, so strive to make this easy for them.
Not surprisingly, what makes a good commit (on master) is basically the same as what makes a good PR. This doesn't mean that you shouldn't make commits that don't follow those rules locally; you should just attempt to clean them up before submitting for PR.
A good commit:
- accomplishes a single, coherent objective (e.g., add a fn and test; add a ns and test; refactor to rename a fn; change behavior to do x)
- begins and ends in a state where the tests all pass
- comes with a descriptive message explaining what the commit accomplishes.
- These should be sufficiently detailed to make sense in a
git log
on master. - "Fix bug" and "WIP" do not count
- Should start with a one-line summary, and can (and often should) contain additional lines or bullets to explain more context about the change. For more details on what makes a good commit message refer to this [blog post] (http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html).
- These should be sufficiently detailed to make sense in a
We're based almost exactly on the GitHub Flow model. Ignore the talk about git-flow in that post, and start at "GitHub Flow". You should probably read this whole document. Reproducing the summary here:
- Anything in the master branch is deployable
- To work on something new, create a descriptively named branch off of master (ie: new-oauth2-scopes)
- Commit to that branch locally and regularly push your work to the same named branch on the server
- When you need feedback or help, or you think the branch is ready for merging, open a pull request
- After someone else has reviewed and signed off on the feature, you can merge it into master (after rebasing it on the latest)
- Once it is merged and pushed to ‘master’, you can and should deploy (at least to staging) immediately, so that bugs and other unanticipated issues are caught as quickly as possible.
The only difference from GitHub is that at Prismatic we currently only deploy to staging after every merge, with periodic prod deploys (at least once a week), but the principal idea is still that production should be deployable at any time from master.
Note that in this model, no code is ever committed to master, only merged from branches. If every branch is rebased on master and merged using --no-ff
(which github does), we get the clean history model shown above.
Our workflow for making branches that follow the rules laid out above are:
-
first
git checkout master
,ppull
to get the latest (a shortcut forgit pull && git submodule update --init --recursive
run at the repo root), and finallygit checkout -b my-awesome-branch
to get your branch started -
Do your work. Commit as early and often as you like, make "WIP" and "bug fixes", do whatever makes you happy
- While working,
git fetch
andgit rebase origin/master
often to keep your branch up-to-date so you don't end up with huge merge conflicts later. - Do not merge master into your branch, since merge commits clutter up history, make it nonlinear, are difficult to remove later, and make it hard to apply post-production to your branch before sending it up for PR.
- While working,
-
When the tests pass and you're ready to ship your code or share it with the rest of the team, make it look nice and follow the rules above. For an overly pedantic treatment of the subject, this is a great resource. Typically, a single
get rebase -i HEAD^N
whereN
is the number of commits on your branch is all you need to reorder and squash commits so that everyting left represents a good state with a reasonable commit message -
Finally, rebase on master one last time and put it up for PR.
-
Address PR comments. If you disagree with a comment, now is the perfect time to have a discussion about the points made, and add an entry in or update the style guide if necessary.
-
If significant time passes before making a PR and its approval, rebase on master one more time; then merge into master.
-
Deploy to staging! And production?!
One point is that if you're collaborating on a branch, you must be careful with rewriting history. You should never rebase work that is shared with someone else, without careful coordination first.
For commits, it should be pretty straightforward to just follow the above rules. Some common gotchas and tools to make this nicer:
- Always use
git status
before committing. It will tell you if you have unsaved files in emacs, or new files you're forgetting to add. - If you forget to put something in a commit or want to change the message,
git commit --amend
is your friend. You can use this in a "commit early, commit often" workflow where you continually amend your commit until it's in tip-top shape. - If you make a bunch of unrelated changes, you can still break them into separate commits. You can use
git add
on individual files to stage your ideal commit, or evengit add -p
to stage changes to parts of files. Usegit diff --staged
to see exactly what you've staged before you commit.
Choose your own adventure on how to fix mistakes:
http://sethrobertson.github.io/GitFixUm/fixup.html
[alias]
lg = log --graph --pretty=format:'%Cred%h%Creset -%C(yellow)%d%Creset %s %Cgreen(%cr) %C(bold blue)<%an>%Creset' --abbrev-commit
lga = log --graph --pretty=format:'%Cred%h%Creset -%C(yellow)%d%Creset %s %Cgreen(%cr) %C(bold blue)<%an>%Creset' --abbrev-commit --all
lgd = log --graph --pretty=format:'%Cred%h%Creset -%C(yellow)%d%Creset %s %Cgreen(%cr) %C(bold blue)<%an>%Creset' --abbrev-commit -p
df = diff --color --color-words --abbrev
[color]
branch = auto
diff = auto
interactive = auto
status = auto
[core]
editor = /Applications/Emacs.app/Contents/MacOS/bin/emacsclient -t -a=\\\"\\\"
[push]
default = simple