Is it fine to have broken intermediate commits, as long as the final commit in any push works?

Question

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

Suppose I make the following commits locally:

• Modify the database schema, breaking the application.

• Update the application so it is compatible with the database schema again.

As long as I push both commits, master remains in a working state. However, a historical version is broken.

I'm aware that I can use git rebase -i to squash the commits together. However, the resulting commit will be large and less descriptive. If I need to search the commit history to find out why I changed something, I'd rather find the original commit showing what I did and why.

My questions are:

• Has anyone encountered difficulties due to broken historical commits in master?

• If so, is there a simple way to avoid such difficulties, without discarding individual commit messages and changes?

Answer 1

Largely depends on your outfit's branching strategy, but I think that having broken commits on development branches makes a ton of sense in general -- the real big "win" in using source control is to be able to roll back small changes and sometimes you are making a bunch of them and you gotta break eggs to make omlets.

The simple way to keep the individual commits without polluting master is to use branches. You can put the breaking/experimental stuff in there so you can have a fine-grained history without polluting the master branch's history.

Answer 2

No, it's not okay.

If you ever did a git bisect (and who doesn't love that killer feature), you know the value of a history where every commit builds.

If you have many commits during a bisect that don't build, you'll have lots of git bisect skips that make it difficult to find the last good commit.

If you finish a feature branch and merge it into master, cleanup the branch before merging so that your history is clear and building.

Answer 3

Broken commits are something that "just happens", shouldn't mean the end of the world. I do have a little nagging voice in the back of my head that tells me one shouldn't knowingly check in broken code, as a matter of principle and therefore including historical versions, however it's not something I'd go to war over.

Git's much praised branching model makes it feasible to keep broken commits off specific branches, e.g. if your team adopts gitflow or a simplified version thereof. Anything with a "clean master" policy. In this case, you could check in the final, working version as a merge commit, where the (broken) historical versions are available in the repository but off the main line.

If your team hasn't adopted such a branching model, then you have a valid excuse to just push the whole lot to master and be done with it.

Answer 4

Has anyone encountered difficulties due to broken historical commits in master?

Yes. Backports, reverts and bisects are harder. So is reading the history (see below).

If so, is there a simple way to avoid such difficulties, without discarding individual commit messages and changes?

Not that I know, although branches are a decent solution.

However, I think that discarding (or rather squashing) the individual commits is the right thing to do.

During development, especially when doing TDD, committing early and often is good. You want the full trace of what you're doing so yo can backtrack, or figure out exactly when things started going wrong (or maybe you got yourself into a refactoring larger than you can chew). Commit away.

However, once a feature/change is ready to be pushed, a commit is a packaged change - ideally atomic so it can be [reviewed, rebased, merged, cherry-picked, reverted, viewed in annotate] as independently from other changes as possible.

Looking at the history of a project, a software change should be evaluated on its own. Does it build? Does it come with tests? Does it work? What does it do? What files needed to be changed to deliver that feature?

Having to assemble commits, while possible (and helped by merges), makes that harder. Therefore, I think cleaning up your history before pushing is appropriate, even if it means loosing track of how you arrived to where you are.

Answer 5

Short Answer: Yes

Testing:

Test driven development means you write tests that break (ie show failing).
Then you write the code to make the tests work.

Development:

Commit small, Commit often.
Each commit is a rollback point. If you realize you are on the wrong path you can rollback to a previous point relatively easily. If you are commits are fine grain enough you can rollback to the correct point.

Caveat:

This does not mean

1) You check broken code into master.
2) You need to push all micro commits for everybody to review.

Use branches to do your work. Potentially compress micro commits for the review processes so they are in logically distinct units with appropriate comments. If you are using git you will not loose the original set of commits you can just create a new more logical set of commits for the review proceses that will be merged into master.

Answer 6

I think as long as the commits are local and not pushed to others it's not only ok, it's actually a good way to work. Just don't push broken code to others.