Policy for committing to Git
Why do we have a commit policy?
SVN only has one way of working: you check out, you make changes, you try and commit, and it fails if anyone else committed before you. You then have to update, resolve conflicts, and try and commit again. This means there's not really any policy element there: you are always committing directly on top of all existing changes.
Git allows virtually an infinite flexibility of workflow: you can rebase, you can merge, you can merge-without-fastforward, you can create patches and email them, you can do various other weird combinations of things. Different git users, and different git-based projects, use different workflows, and not always the same workflow all the time. This is a feature, of course: being limited to SVN's strict only-commit-on-top workflow is a pain sometimes. It's also a potential problem for other people that work on the project: if you manage to create some particularly weird shaped and weird looking history, you can end up pushing something to the server that confuses other developers greatly (as well as users trying to bisect revisions).
So, in the interests of everyone's sanity, it's good to have some best practises. They are not meant to apply to 100% of cases, but they should only be ignored when you have a really good reason, and ideally when other people also concur that you have a really good reason.
Also, users who are new to git may find the lack of workflow constraints to be difficult: they may get different advice on how to work depending who they ask, and so it helps to have one suggestion to
refer people to.
The policy
Throughout this section, when we refer to "origin/master" we mean the canonical copy of master on the Rockbox server, not any branch named "master" in any other remote repository.
- If you started with some version of origin/master, and then made a local commit, and then when you try to push you find you are outdated: please rebase your local branch onto the latest version of origin/master. Avoid merging origin/master into your branch.
- If you started with some version of origin/master, and then made multiple local commits in a series: consider whether you should flatten the commits before pushing. A good indicator that a series of commits should probably be flattened into a single commit is if the intermediate states do not compile or work, or if you cannot think of any way to describe them individually other than "part N of M". The goal is for each commit to represent a logical change: if your overall change is divided into three logical steps, feel free to make three commits with appropriate titles and descriptions, but if your three commits are "the changes I made on Monday, the changes I made on Tuesday, and the changes I made on Wednesday" then it is likely that the intermediate states are interesting to nobody, not even you in the future, and flattening them is a clear improvement.
- If you started with some version of origin/master, made some number of local commits, and now want to update your branch to a newer version of origin/master without submitting: rebase your local branch onto the latest version of origin/master. Avoid merging origin/master into your branch.
- If you publish a branch for collaboration with other people:
- Don't rebase that branch while working on it. Rebasing a branch you have published means everyone who has a copy of that branch needs to manually fix things by rebasing their own work onto your rebased changes, and nobody will be happy about this. In order to get new changes from origin/master into a published branch, you will need to merge origin/master into your branch.
- When the work in the branch is done and you are ready to push back to origin/master: please consider rewriting the branch (some combination of flattening and rebasing), rather than just pushing it as-is. The same logic as in 2. applies: it's nicer if the changes that end up on origin/master represent logical steps, and it's likely that many of the intermediate states are not interesting to anyone now that the change is complete. However, this can be substantially more difficult, depending exactly what changes occurred. It's always possible to simply flatten the entire branch to a single commit, but that may remove too much information. More complex rewrites are often hard. If you are sufficiently confident of your git abilities, please do attempt it, but it is okay to admit defeat and leave the branch either as-is, or in a partially rewritten state which still contains merges. Before pushing a branch in this condition, talk to other committers about it.
Rationale for the rules
- If you have just made one change to the branch, then either that change conflicts with the latest version, in which case both a rebase and a merge will require you to resolve those commits by hand identically, or that change does not conflict with the latest version, in which case a rebase will succeed with no manual intervention. There is no extra effort for you, and the result is a cleaner history which does not contain a pointless merge commit and is easy to read due to its linearity.
- Making multiple local commits is perfectly sensible and a good way to track any work that you don't complete in a single session, but often once the work is complete the split into multiple commits is arbitrary and the intermediate states are useless. Flattening into a single commit is trivial and makes history simpler for everyone in future. Reworking a set of N commits into a different set of M commits is more effort, but can be done to turn a series of time-based commits into a logical series.
- This is the same case as 1. If your work lives in a private branch on your local repository, there is rarely a reason to prefer merging to rebasing.
- History cleanliness is strongly preferred in cases where it is no more effort than merging, but lack of any sensible way to reach a clean history should not block a commit. If nobody sees a sane way to make things clean, then everyone will have to tolerate a patch of more complex history which contains merges.
Enforcement
There is soft technical enforcement of this policy. By default, Gerrit (our git hosting platform) will not allow you to push a merge commit to origin/master; you will be forced to rebase before you can submit. This serves two ends: helping committers who are new to git avoid complicated mistakes, and encouraging committers who are experienced with git and normally use a different workflow to seek out the policy document and review it.
There is a Gerrit user group called "Rockbox Merge Committers" containing users who are allowed to push merge commits. Any committer may add themselves to the group (or ask another committer to do it for them, if they don't know how to navigate Gerrit). The only restriction on joining the group is that we ask you to read and consider this policy document first, and use your power of merging wisely. It's also probably a good idea to remove yourself from the group once you are done.
Copyright © by the contributing authors.