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...
This workflow gives good results for most large software projects that follow some version of the the Atlassian Git Flow model
Each merge to the branch from which the release is cut must leave the project in a working state. In Git Flow, this is called the master branch, and usually only the release engineers can merge code there. Run all your tests for each merge;
Each merge to the mainline development branch (in Git Flow it's called develop branch, but many use the master branch for that purpose) should leave the project in a working state (and it must build at least). Run most of your tests, except the longest running ones, accept some flakiness, don't run unaffected tests. But sometimes you'll merge two branches that both pass tests, but their merge is broken. This is okay. You'll fix this soon, and you'll make sure your release branches are good.
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.
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.
rebase which is quite controversial: Thou Shalt Not Lie: git rebase, amend, squash, and other lies
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.
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.
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).
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.
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.
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.
