Should every git commit leave the project in a working state?

Question

I'm curious to know what the prevailing best practice is about get commits are. Should they be enforced such that the project is in a working state (builds properly, all tests pass etc), or is committing broken code OK?

If you waive the requirement, you can be more flexible with commits (use them as logical chunks, even though the app is not in a working state etc). But if you enforce it you gain the flexibility of being able to cherry-pick any given commit later on...

Answer 1

Use your local clone of the repository for whatever makes you comfortable while developing.

I commit broken code regularly, and when I am ready to make the code available to other developers, I use a great feature:

git rebase -i HEAD~4

This allows me to compress my intermediate (in this case 4 of them), possibly broken, commits into one good commit. You will be presented with an editor allowing you to choose to choose how those commits are compacted. Typically I marked the first commit the 'pick' commit, and mark the others 'squash'.

Then I can push that one atomic commit, or in fact what I do if my new feature is really ready, is use 'git cvsexportcommit' to get my work into the existing CVS repo.

Answer 2

I usually follow this rule:

• Each merge to master branch must leave the project in a working state;
• Each merge to mainline develop branch should leave the project in a working state (and it must build at least);
• Each other individual commit has a primary goal of explaining why the change is made, and what is it for, and what parts of the project it affected. All other goals, such as leaving the project in a working state, are optional.

Answer 3

We generally follow both approaches. In the local repository on my box I commit all I want. When it's time to push to my team's central repo, I first do an interactive rebase and mold my commits into logical packages. Typically one commit per story, with the story (or defect) id included in the comment (we are a kanban based shop).

Then on our central repro we have Jenkins listening and it kicks off the build and all tests. If anything fails, we generally allow people to try and get the build fixed with another commit. If it's not looking good, reverting the faulty commit is easy to do.

Answer 4

Assuming you are using branches, and good commit messages, committing "broken" code on a branch with a commit message that makes that clear would be fine, so long as your team agree that it's a good working practice.

You also clone the git repository locally, so you might well just have a local branch with local commits not pushed to origin where you're committing "broken" code as you go along; then, when you have it all working, you could merge it in to master or some other branch, and delete your working branch with the various "broken" commits.

To me, it's all about agreeing with your team what is acceptable; some teams won't accept broken code even on a branch, others will.

Answer 5

Since git commit only affects your own copy of the repository, there's no need for the project to be in a working state after every commit. Go ahead and commit whenever you want to save the work you've done. Probably a good rule of thumb is that a commit is appropriate when you can describe the changes you've made in the commit message.

It's git push that affects other users. Policies for what should be pushed are a matter for your development team to decide. Pushing non-working code onto the main branch is presumably a no-no, but it's probably ok to push non-working code onto a separate branch (as long as nobody else is going to try to do a build from that branch).

Answer 6

We are using git flow at work, and we also commit unfinished or broken code - as it only lands in local or remote branches made for that specific problem. Only once the task is finished, it gets merged into the develop branch (which represents the current working copy in the flow model). That way, we also can collaborate on the code (some coworkers are in another city, including the project lead) and help each other.

However, it depends on how you and your coworkers are thinking. Personally, I think branch commits are okay, as you might need a history of changes with a bigger refactor or similar.

Answer 7

Two of the great benefits of version control are that it lets developers recover previous versions of their work, and it lets developers try different, possibly conflicting changes at the same time. Version control gives developers the freedom to try ideas that might fail.

Developers should be encouraged to branch and commit their work regularly, whether it builds or not. To refuse to allow branching or broken commits is to hamstring your developers and make poor use of your tools.

That said, it's an excellent practice to require that commits to certain branches always build. Many organizations go further and prohibit developers from committing to certain branches at all. For example, developers might be required to merge their work back to the main development branch, but only the lead developer might be allowed to merge those changes from development to the production branch.