Skip to content

Latest commit

 

History

History
142 lines (88 loc) · 8.83 KB

20140403-git.md

File metadata and controls

142 lines (88 loc) · 8.83 KB

Prismatic and Git

What do we like?

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.

History

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

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.

Commits

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).

How do we make nice things?

Clean History

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.

Branches/PRs

Our workflow for making branches that follow the rules laid out above are:

  • first git checkout master, ppull to get the latest (including submodules), and finally git 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 and git 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.
  • 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 where N 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.

Commits

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 even git add -p to stage changes to parts of files. Use git diff --staged to see exactly what you've staged before you commit.

Elsewhere in the doc?

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