commit_discipline.md

Master Branch Commit Discipline

1. Every commit must be fully human tested before hitting the repo.

You must be able to confidently say: “This commit does what it’s supposed to do, and does not either contain obvious regressions or introduce new bugs”. You have proven this to be true because you have tested the commit in the app (or browser or other relevant client), whilst also testing all reasonable combinations of related functionality to ensure there are no obvious regressions.

There should be no surprises. Regressions and new bugs should be limited to rare edge cases and unexpected conditions that fall outside of the range of normal user interaction with the app.

2. Each commit should be considered complete, in the sense that is a functional, tested, and shippable chunk of work.

Note that this does not mean that the commit needs to contain a completed feature, nor a completed sub-unit of a feature. It might only contain a single line change. But it must be confirmed that it leaves the product in a state where the product can continue to be shipped to the App Store or demoed to the customer without fear of surprises. Treat every commit as a product release.

3. Do not branch. Push all commits to the master branch, and rebase over other people’s incoming commits repeatedly throughout the day.

   • Exception 1: Create a branch if your chunk of work requires unavoidable regressions that cannot be managed in a single commit.

     • Note: The branch should be merged back into master before the end of the day. Exceptions to this rule should be rare, and require explicit justification.

   • Exception 2: Create a branch if your chunk of work is an optional feature that might not be included in the release, and cannot be cleanly integrated with other code that assumes the absence of said feature.

4. Push to master without review. Code reviews will be performed post commit, per commit, on the master branch.

5. Review all commits to the repo by other developers. Each time you pull and rebase the repo, skim through the new commits from other developers and briefly examine them for correctness, and so that you understand the changes that have been made. All devs on the team should be reviewing all other dev’s commits throughout the day, on a per commit basis.

Rationale

• Treating each commit as a self contained work item and product release encourages coding and testing discipline, and ensures a more stable and human tested product. No single commit should be able to knowingly break or regress the product.

• Reviewing individual commits is easier and faster than reviewing multi-commit branches, and can thus be done incrementally throughout the day, for example whilst waiting for a compile to complete.

• Reviewing and testing multi-commit branches (eg PRs) increases the risk of missing incremental details, due to the greater size of the review task and greater amount of changes to parse and understand in one sitting. It also increases the time required for each review task, thus often resulting in code reviews being deferred, happening later rather than earlier.

• Post commit reviews allow the repo to continue evolving at pace, without pause, instead of causing process blockages and bottlenecks on completion of each item or sub-item.

• Merging branches (eg PRs) is more complex and error prone than rebasing over single commits, increasing the chances of unexpected and non visible regressions at merge time, which can more easily slip through code review, and fail to show up during the dev’s own human testing.

• Working in branches permits and encourages temporary regressions in individual commits, and breeds the habit of deferring human testing until the branch is ready for merge. Issues and regressions in the branch are discovered later rather than earlier, and mistakes become more difficult to correct at merge time.

• Post commit reviews on master foster the habit of all devs reviewing all commits, and performing the reviews earlier rather than later. Thus resulting in bugs and regressions being caught earlier, and devs having greater and more timely awareness and understanding of the changes performed by other team members.

• Because each commit is being pulled and rebased over by other devs earlier rather than later, each change receives more human testing by more of the team.

  For example if a change is made to the Sign In view in the morning, as part of a larger incomplete change, other devs will have that commit pulled locally in the morning, and be implicitly human testing it tens or hundreds of times throughout the day, whilst testing their own changes. If there is an issue in that commit, the dev responsible for it can be alerted earlier, and fix it earlier, rather than finding out only at the end of the day when they come to submit their PR.

• Because all commits are going to master, the master branch is never in an out of date state, and thus always represents the latest shippable and stable product.

  Whereas when chunks of work are living in unmerged branches, at any point during the day (and sometimes even across multiple days), the master branch will not an honest representation of the project state and stability. This results in a greater number of unknowns throughout the day, a greater number of deferred and undiscovered bugs and regressions, and each dev having less awareness of the project’s current overall state.