Better Git Workflow

better-git-workflow.md

A good pull request:

• Is easy to effectively code review
• Is easy to effectively QA
• Adds quality code and useful information to our codebase

This is primarily accomplished by keeping PRs and commits small, well described and single purpose.

Why bother?

PRs

• Small PRs allow review to move faster and more effectively

  • A 50 line PR gets 10 "Can you change this"'s
  • A 5000 line PR gets a "LGTM"

• Small PRs get QA'd more confidently

• Small PRs get released more often and with more certainty

  • Keep forward momentum
  • Less time set managing PRs, merging, fixing merge conflicts, after you've already context switched to a new task

• BUT we make big changes. A large PR is still a good PR when it is composed of small, well described commits.

Commits

Commits are what matters in git. Good commits make git a real tool.

• Small, well described commits save review time and increase review quality

  • Communicate context and thought process to the reviewer, preventing unnecessary back and forth
  • Focus of a review decreases in direct correlation to the complexity of each commit
    • A commit that makes the same change across 100 files is very easy to review. A commit that makes three changes across three files is very difficult to review.
  • Small commits are easily amended or removed during code review

• Small, well described commits save development time and improve development happiness

  • git history provides a resource for future developers, including yourself (the author of the commit)
  • git blame is one of the most powerful tools we have to understanding why a piece of code is in the codebase

• Small, well described commits teach us to think and work more efficiently

  • Changing one thing at a time prevents debugging rabbit holes
  • Changing one thing at a time enforces scope

• Small, well described commits help us move faster

  • Give us more opportunity to see where we can stop and deploy. Sometimes as we work on a code change we realize that it's actually a collection of smaller, independent changes that can be cherry picked or split into multiple PRs. Back to point one, small PRs FTW!

Let's jump back to the idea of better reviews and a well described history.

Re-establishing the context of a piece of code is wasteful. We can't avoid it completely, so our efforts should go to reducing it [as much] as possible. Commit messages can do exactly that and as a result, a commit message shows whether a developer is a good collaborator.

Small, communicative commits are asynchronous collaboration.

OK, let's look at some good examples:

Keep commits single purpose

Small commits clearly show the author's intent.

Tell us what was changed and why it was changed. Avoid describing how you changed something; that information is recorded in your code.

Each commit should be a single, self contained idea. Some examples:

• Add a piece of functionality
• Implement a visual change
• Bump dependencies
• Remove dead code

Some teams insist that the application be functional at each commit (while still explicitly avoiding "chunky", multi-change commits). Others are more interested in moving quickly while building a good resource for their future; as long as the PR works, some non-functional states are OK. I lean towards the latter, but it's good to keep the former in mind.

Patterns are important for understandability. They reduce cognitive load. There are widespread patterns for git commit messages already: http://chris.beams.io/posts/git-commit/#seven-rules

Good:

Allow users to close all notifications
Fix ESLint errors in routes controller

Remove unnecessary fastclick.js

We thought we needed fastclick to reduce the tap delay on mobile devices, but it turns out all the mobile browsers we support recognize `touch-action`. 

References: https://site.com/article-about-stuff.html

Not so good:

added new removeUser method
fixed stuff we talked about irl
document behavior
fix last commit

Pro Tip: Make sure your message makes sense outside the context of your individual PR.

Should I squash?

As a rule, don't reduce the amount of information available by squashing commits.

Squashing commits that do the same thing (multiple whitespace, related refactor, "start" and "end" a single change, removing a log or debug statement) is good. It reduces Chatter but not Information

For more info on good commit messages, check out this excellent article by Chris Beams.

For more info on small and fast PRs see How to level up as a developer.