Is it ever OK to commit non-working code?

Question

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?

---

Some of the highly voted comments say 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.

Answer 1

One of the philosophies suggested by Linus Torvalds is that creative programming should be like a series of experiments. You have an idea, and follow it. It doesn't always work out, but at least you tried it. You want to encourage developers to try creative ideas, and to do that, it must be cheap to try that experiment, and cheap to recover. This is the true power of git commits being so cheap (fast and easy). It opens up this creative paradigm that empowers developers to try thing they might not have otherwise. This is the liberation of git.

Answer 2

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.

Answer 3

Yes, so long as it is not a release branch.

In personal branches, everything goes and can then be discarded if the experiment did not work. That is one of the main benefits of DVCS: freedom

The value of committing broken code?: collaboration and experimentation

Answer 4

Yes it is OK and its something I do a lot.

The point of committing non-compilable code (in branches at least) is that sometimes your code is a work-in-progress, but that the work done so far is worth saving and/or sharing with others

My Practices are:

• write tests first
• wip (work-in-progress) commits are good
• commit often (multiple within a day) and early (save 'working' steps)
• push after every commit (in case your hard drives crashes / the bus hits you).
• always work in branches first
• when possible only merge in working code to master
• interactive rebase in git to squash wips before master merge

The main issue and perhaps the one you are touching on is when you have a feature that basically works and is sorely needed by the business (and hence needs to be in 'master') but has some failing tests. One option here may be to do a pending test that lets you move forward for now. However this is fraught with danger as the test may never be fixed and it may set a pattern in other areas of simply 'pending out' broken tests instead of fixing them.

Another option would be to temporarily use and deploy the branch. This can help in certain situations but is generally not recommended and is not sustainable.

Perhaps the best option is to basically take a more professional approach to software development and really require working tests for any committed code. This is frequently the 'hard' part of software development, not the coding that many people imagine. A better approach will likely require better initial estimates, resource allocation, priority setting, etc. plus, during Agile development, allowing enough time and using enough discipline to fix any issues both at the time they occur and during grooming - pointing sessions.

Focus on what 'done' means - it means the code AND the tests are written, have been refactored and work. If you hear comments such as "mostly done, just need to write / fix / refactor tests, then it is NOT done. Saying a feature is done without it being technically complete is one of the most common mistakes of junior programmers.

Answer 5

Before you start getting dogmatic about how to work with your version control, it's worth thinking about why you're working with version control.

Committing to version control freezes the state of your code for future reference - everything else is falls out of this. Looking at diffs and making patches is just seeing how code changed between snapshots. Branches and tags are just ways of organizing snapshots. Sharing code with other developers is just letting them look at a particular snapshot.

When should you commit? When there's a reasonable chance you will look at the state of your code (or the commit message explaining a change) in the future.

Git gives you a whole lot of flexibility as to how to organize your snapshots. There's no central repository so you can share your code with other devs without pushing your state to the 'main' repository. You can easily create, merge & delete branches to isolate the details of a set of states from the main code's narrative. You can commit locally, to help you undo a follow your current development, and then batch everything up into a single commit before pushing it out for others to see. You can tag specific revisions so they're easy to find later.

KISS. What works best for a single developer in the early stages of development of a small project is going to be completely different than what you need to do when you have a hundred developers working on a decade-old, mission critical system. In any software development process, you should avoid creating unnecessary artifacts simply because somebody else told you to do it.

Answer 6

Think about it this way. As a developer one of the most disruptive things you an do is to stop other developers on your team from working on their tasks.

The philosophy of only committing working code comes from development teams working on the same single trunk in the repository. It may seem madness now, but 10 years ago, this was the normal way of working. A branch would appear when you wanted to create a stable release, but the thought of a developer working in a branch to implement a new feature was almost unheard of.

If your environment means that your commits do not immediately affect other developers, then commit often. it gives you more security in your code, making it easier to roll back a code mistake and many source control systems give you some code protection to committed code (although not all).

Now, making sure your merges with branches shared with other developers do work, and that any code you promote to this level compile, pass all unit tests and other team based sanity checks... that's kind of essential if you don't want to keep buying the beer in the pub...

Answer 7

The value of any commit, broken or not, is that the code is commited to a server. In professional environments, that server is secure, redundant and running backups. If I work all day, commiting is a form of making sure my code survives whatever happens to my local machine. Hard disks die. Laptops get lost or stolen. Backups of the repository server will be available even if the building burns down.

Answer 8

Build/Release Branches

You should never deliberately commit broken code to a build branch. Any branch that is under continuous integration or from which releases or daily builds are made should always be in a potentially-releasable state.

Other Branches: Save State Often

For private or feature branches, the goals are often different. Frequent check-in of code (whether working or not) may be desirable. Generally, you will want to commit any time you may need to rewind to the current state.

Consider these examples where saved state provides a significant benefit:

• You might perform a commit right before you run a global search-and-replace so you can revert your tree in a single operation if things go wrong.
• You might perform a series of interim commits while refactoring a complex piece of code so that you can bisect or rewind if you end up breaking something in the process.
• You might perform a commit, start a new branch, or create a tag when you want to try something experimental while being able to return to the state of the current working tree at any time.

Answer 9

I don't think that it's OK to commit broken code.

What happens if

• An urgent hot fix is required. Code base is in a broken state. You are forced to roll back, fix and deploy.

• Somebody else starts working in the same branch not knowing that you have committed broken code. They might be chasing a 'red herring' thinking that their changes have broken something.

• You decide to leave the company, go on holiday or can't come to work for any reason. Your colleagues will have to dig deep to find what's broken and why it has been committed in a broken state.

• Somebody deploys your 'broken code'? This can be a 'game over' if you are working with personal data or on a payment provider.

Reply to @WarrenT

I agree with you that in an ideal world where everybody works in a feature branch, committing non working code might work. I have worked on large projects and even then there were cases where multiple people had to work in a single feature branch. I have also seen people commit 'not working' code to main branch because release was weeks away and they planned on fixing it next day. All these things are candidates for a disaster and I strongly believe that they should be avoided at all costs.

Answer 10

Committing some broken code-base is okay as long as it is local.

Why?

• It is essential to use committing as a save point in your development
• It shows you a pattern of thought used while developing the product.
• It does not break collaboration.

However, when there is a team of programmers, the philosophy of the programming house is paramount and supersedes individual commit behaviours. Some programming houses decide to log all progress while others decide to only commit code that solves a feature. In this case, the value (cost, from a software management point of view) of a broken commit is dire:

1. time used to further features is now spent fixing errors...
2. the development cut is not met...
3. product is not shipped on time

Other points can be added to these three cascading their effects exponentially into a company meltdown ...of course, this has to be an effect of chronic habitual committing of bad code.