50

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.

10
  • 35
    In local branches everything goes. Commit whatever you want. Just clean up before you push. Sep 16, 2013 at 9:32
  • @Joachim Sauer, I am asking about best practices and reasoning behind them, in order to build up good habits. Currently I often commit broken code. And reverting is a nightmare, through the tens of commits for the past couple of days.
    – Vorac
    Sep 16, 2013 at 9:52
  • 10
    if you develop each feature on its own branch, then that decision is trivial: discard the branch, continue from a new branch created on the current master Sep 16, 2013 at 10:44
  • 1
    unless you define what makes commit "broken", it is impossible to authoritatively answer this question. See also: Should a programmer fix someone else's failed build?
    – gnat
    Sep 16, 2013 at 12:07
  • 2
    "Why? What is the value of a broken commit?" The ability to have identified a problem and test multiple different resolutions on top of it and to be able to reliably go back to the problem that you knew you had, rather than a state where you've got that and maybe some new ones, too. Apr 13, 2015 at 15:43

10 Answers 10

26

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.

46

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.

4
  • 3
    It is indeed a beautiful thing. I don't think the community has truly learned to appreciate git and DVCS in general. Sep 16, 2013 at 17:18
  • 5
    As long as people don't confuse this philosohy with trial and error programming, which should be avoided.
    – Pieter B
    Sep 17, 2013 at 7:02
  • @PieterB how is that different? Oct 6, 2022 at 9:30
  • Experimentation is trial and error. That's exactly what he was recommending. You have an idea. Try it out. It may have errors. You might fix them, or you might decide it's more feasible to do it another way. When it works, you have expanded your horizons, and hopefully improved the code with a better way of doing something. That's the purpose.
    – WarrenT
    Mar 16 at 13:55
14

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

1
  • Minor tweak: Yes, so long as it's not going to be run in production. Code that's hidden by a feature toggle, for example. Apr 7, 2017 at 14:56
9

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.

6

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.

3
  • Your answer is inspiring, but vague to me. My problem at hand is that I create (I think) too many commits, and when I need to revert, I do not know to where, as many of them do not work. The context is solo development of small team of <5. I think I am taking the "commit often" thing too far away and may need to recover. How do I do commits, that will have meaningful commit message?
    – Vorac
    Sep 17, 2013 at 7:42
  • 2
    If you find you have to commit every time you compile/test, maybe you need to find a better editor that gives you more undo history. If you can't write a commit message that meaningfully expresses your changes, don't commit. Sep 17, 2013 at 16:46
  • If you're having trouble remembering what commit to go back to, you're not merging your changes often enough. Keep a "stable" branch, a "development" branch and an "experimental" branch. When the code is working, merge your changes from "experimental" into "dev" (squash your commits first), then feel free to break "experimental" knowing you can just delete it and create a new branch from "dev". Once an entire feature is completed merge dev into stable.
    – RubberDuck
    Apr 14, 2015 at 1:08
4

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

3

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.

7
  • 9
    This isn't necessarily true for DVCSes. For example, if your local personal repository or feature branch is local, it may or may not be backed up (and this is especially true if you're working offline from the corporate network without access to company resources). The master and release branches should be somewhere that is backed up, but that's not necessarily true for a local branch.
    – Thomas Owens
    Sep 16, 2013 at 9:47
  • Even on DVCSes a commit is worth something, as the code in it is "more permanent" then the one in the files. At least for git. Sep 16, 2013 at 10:09
  • 3
    @ThomasOwens, with all due respect, if your DVCS admin set your system up so that local repositories or branches are not backed up, your DVCS admin is an idiot and needs to find a new job more suited to his talents ("Would you like fries with that?"). If your DVCS admin did it that way because your IT guys told him to, that goes for your IT organization as well. If anything, this is arguably an indictment of the entire DVCS concept: committing code to VCS should BY DEFINITION mean committing it to automatic backup. Sep 16, 2013 at 11:59
  • 7
    @JohnR.Strohm When traveling, I often have limited network access, so I don't have access to any of the backup resources or network drives. I'm developing without backup until I have access to a network. This is the point of a DVCS - you don't need the network. Should your repo be backed up? Probably, but it's only a requirement for release or master repos. You really shouldn't go days between pushing to a backed up repo anyway, but those pushes should not be broken code.
    – Thomas Owens
    Sep 16, 2013 at 12:06
  • 1
    @ThomasOwens my point is that although your access to said backup server is only sporadic, committing to it should be a priority over making your code work. You can always make your code work on another machine, even other people on your team could make the code work on their machines. If it sits on your machine only, the customer will most likely not care if it works. The customer cares for a new release from the build server that in turn draws from the server repository.
    – nvoigt
    Sep 16, 2013 at 13:29
2

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.
1
  • Wanting to be able to use bisect is normally a reason to avoid keeping commits of non-working versions - i.e. versions where you're aware of any big new bug that you plan to fix very soon, or where the build fails. Then you can use bisect to see when if ever you introduced a bug you weren't aware of.
    – bdsl
    Dec 3, 2022 at 11:53
0

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.

0
0

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.

4
  • Downvote. None of these scenarios are likely if you're using a standard DVCS workflow.
    – user16764
    Sep 16, 2013 at 22:05
  • 1
    With git (or other DVCS) workflow, developers are working on branches, local branches, NOT the main trunk. The next question would be should a developer ever push broken code to another repository. This is a matter of a team having an understanding of what branches are for what, and of using good comments on your commits. A hot fix would only be made on a release branch. Of course nobody should be merging broken code there. Teams do need to have rules on workflow, but what is done in a local repository is an entirely different matter.
    – WarrenT
    Sep 16, 2013 at 22:58
  • I think there's a difference between "non-working code" (what the OP asked) and "broken code" to which you refer.
    – Simon
    Sep 17, 2013 at 6:12
  • @WarrenT Please see reply.
    – CodeART
    Sep 17, 2013 at 7:20

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