Version Control – Is It Ever OK to Commit Non-Working Code?

branchinggitversion control

Is it good idea to require to commit only working code?

This commit doesn't need to leave the repository in a working state as:

  • … we are in early design stages, the code is not yet stable.

  • … you are the sole developer on the project. You know why things aren't working. Furthermore, you are not stopping anyone's work by committing broken code.

  • … the code currently doesn't work. We are going to make a big change to it. Let's commit, in order to have a point to revert to if things get ugly.

  • … the chain is long, no trouble if broken code exists in the local branch. I.e.

    1. local files
    2. staging area
    3. commits in local branch
    4. commits in remote personal feature branch
    5. merge with remote develop branch
    6. merge with remote master branch
    7. merge with remote release branch

  • … commit early,commit often.

So in the above-linked question, the majority of answers say that committing not-compilable code is no problem in local and feature branches. Why? What is the value of a broken commit?

Added:
There are a couple of highly-voted comments, saying that on a local brach one can do whatever one wants. However, I am not interested in the technical side of the question. Rather, I would like to learn the best practices – the habits, that people who have worked many years in the industry, have found most productive.

I am amazed at the vast amount of great answers! They lead me to the conclusion that I am not adept enough at using branches to organize my code.

Best Answer

One of the branching philosophies (section Developing Branching Strategy and Codeline Policy in Advanced SCM Branching Strategies - also read Perforce Best practices, its a pdf but goes into some other details) is that you branch on incompatabile policy.

A codeline policy specifies the fair use and permissible check-ins for the codeline, and is the essential user’s manual for codeline SCM. For example, the policy of a development codeline should state that it isn’t for release; likewise, the policy of a release codeline should limit changes to approved bug fixes. The policy can also describe how to document changes being checked in, what review is needed, what testing is required, and the expectations of codeline stability after check-ins. A policy is a critical component for a documented, enforceable software development process, and a codeline without a policy, from an SCM point of view, is out of control.

(from Perforce Best Practices)

Say you have the branches 'release' (or 'master') from which a release is built and 'trunk' (or 'dev') where developers check in working code. These are the policies of the branches. Noting the 'working code' is part of the 'dev' branch policy, one should never commit broken code to the dev branch. Often there are things such as CI servers hooked up to these branches and checking in broken code into dev could mess up everyone's branch and break the build.

However, there are times when it is appropriate to check in partial code that doesn't work. In these instances, one should branch - an incompatible policy with trunk. In this new branch, one can decide the policy ('broken code is ok') and then commit code to it.

There is one simple rule to determine if a codeline should be branched: it should be branched when its users need different check-in policies. For example, a product release group may need a check-in policy that enforces rigorous testing, whereas a development team may need a policy that allows frequent check-ins of partially tested changes. This policy divergence calls for a codeline branch. When one development group doesn’t wish to see another

(from Perforce Best Practices)

Realize that this is coming from a central server based SCM with a strong corporate mindset. The core idea is still good. These are often thought of implicitly - you don't check in untested dev code into the release branch. Thats a policy.

So branch, say that this branch can have broken code and commit away.

Related Topic