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

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


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.
| improve this answer | |
  • 1
    We instored the same tules at our office, and this is working fine. Not that this is not limited to git but works with any similar tool (mercurial, svn, etc . . .) – deadalnix Sep 18 '11 at 14:19
  • I hear some organizations like facebook have pipelines which test each commit to ensure they are in a working state – vi_ral Apr 30 at 23:12

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

| improve this answer | |
  • 3
    I question the wisdom of this answer since it relies on rebase which is quite controversial: Thou Shalt Not Lie: git rebase, amend, squash, and other lies – Sled Jan 16 '13 at 16:21
  • 9
    @ArtB: But in this case, memetech is only lying to himself (IOW not rewriting public history), and that is very much not controversial. – Jörg W Mittag May 26 '13 at 14:08
  • 5
    @ArtB Article is referring to published commits. Answers refers to unpublished commits. – d0001 Dec 3 '14 at 16:54
  • 2
    @WayneConrad "a good rule of thumb is that you should not rewrite history for things that are already push out into the world. This would limit these rewriting tools to uses locally to "fix" things up before pushing them." From the last paragraph of the epilogue. – Andrew Oct 17 '17 at 0:36
  • 10
    @ArtB - I question the wisdom of believing everything you read on the internet and doing (or not doing) anything you read on the internet without understanding why (or why not). – mattnz Oct 17 '17 at 2:52

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.

| improve this answer | |

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.

| improve this answer | |

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

| improve this answer | |

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.

| improve this answer | |

Ultimately it's up to you and the people you work with or for, since git doesn't impose any rules.

My practice is to avoid any commit that intentionally makes the system significantly worse. Each commit should either be refactoring or implement some requirement. If I make a bad commit and discover it before I push it then I will amend or rebase to remove it from history.

I think this is makes it easier to read the git log in a pull request, since each commit should stand on its own as either a refactoring or an implementation of some requirement. Adding dead code that will be brought to life in the near future counts as a refactoring. These are my 'logical chunks'.

You can still be flexible with how you structure your commits. For instance you can write tests in advance but mark them all as skipped in the first commit, so that your test suite doesn't report a failure, and then un-skip them when the implementation is done.

| improve this answer | |

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.

| improve this answer | |

Not the answer you're looking for? Browse other questions tagged or ask your own question.