37

Sometimes I hear people saying something like "All committed code must be working". In some articles people even write descriptions how to create svn or git hooks that compile and test code before commit.

In my company we usually create one branch for a feature, and one programmer usually works in this branch. I often (1 per 100, I think and as I think with good reason) do non-compilable commits. It seems to me that requirement of "always compilable/stable" commits conflicts with the idea of frequent commits. A programmer would rather make one commit in a week than test the whole project's stability/compilability ten times a day. For only compilable code I use tags and some selected branches (trunk etc).

I see these reasons to commit not fully working or not compilable code:

  1. If I develop a new feature, it is hard to make it work writing a few lines of code.
  2. If I am editing a feature, it is again sometimes hard to keep code working every time.
  3. If I am changing some function's prototype or interface, I would also make hundreds of changes, not mechanical changes, but intellectual. Sometimes one of them could cause me to carry out hundreds of commits (but if I want all commits to be stable I should commit 1 time instead of 100).

In all these cases to make stable commits I would make commits containing many-many-many changes and it will be very-very-very hard to find out "What happened in this commit?".

Another aspect of this problem is that compiling code gives no guarantee of proper working.

So is it good idea to require every commit to be stable/compilable?

Does it depends on branching model or CVS?

In your company, is it forbidden to make non compilable commits? Is it (and why) a bad idea to use only selected branches (including trunk) and tags for stable versions?

4
  • 1
    Do you now want an answer regarding only committing working code or code that compiles?
    – ChrisBD
    Nov 14, 2011 at 12:23
  • 1
    Depends on the version control type as well imho: for example, in SVN I would want only working code [but in practice, that's not always the case], but in Git, I don't care what you commit or push [to your branches] as long as you push working code when you merge your branch with the development [or feature] branch.
    – wildpeaks
    Nov 15, 2011 at 5:53
  • 5
    Why can't you have your code in a compilable state when needing to commit? Missing }'s?
    – user1249
    Nov 15, 2011 at 8:27
  • I wonder why this question is marked as duplicate of another question which was asked almost after 2 years after this one. Oct 17, 2020 at 18:00

9 Answers 9

43

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

A: It depends

You should have at least 3 kinds of branches in your repository (although sometimes 2 of those are the same).

Production : The code residing in a branch destined for production should always work (as in compiles and passes all the tests). Committing stuff that doesn't pass the tests is a grave offense and steps should be taken so that it never happens (daily, hourly, etc. builds with tests). Code should compile and pass tests.

Latest : This branch holds the latest stable code. This is where every new feature should start from and end in. This is also the branch that holds the code that will eventually make its way into the production branch. Code should compile and pass tests.

Feature : Anything can happen in feature branches. You could be in the middle of a big refactoring or writing a lot of new code. You don't want to lose any of that work so you commit often, even stuff that doesn't work. However, if you're working on a feature with multiple people you might want to set up some ground rules such as code should compile or decide that after a certain point, the branch should be stabilized and tests should pass thereafter. Code does not have to compile or pass the tests (unless decided by the team working on the feature).


For the sake of simplicity, I used commit in my text above, but this also applies to the commit/push combo of distributed version control systems.

1
  • With Git, I don't commit broken code even on feature branches, unless that commit is temporary and is shortly removed (and never pushed to a remote). My usual policy is that every commit represents a known good state of the application. This allows me to work with confidence. If I committed broken code, I wouldn't easily be able to revert to a state that I know works. May 20, 2018 at 16:29
16

I would say it depends on the branch where the commit is made. A feature branch may well contain broken commits, but anything committed or merged to a master or a maintenance branch should compile and pass the testsuite (if there is one).

8

Sometimes I hear people saying something like "All committed code must be working".

That's a fine rule, but it should be applied with some common sense. Rules for what's acceptable to commit should depend on where you're committing the code.

It's common to use a branching strategy that includes some or all of the following:

  • developer-specific branches
  • task-specific branches
  • a main development branch
  • release candidate branches
  • testing branches
  • a production branch
  • version branches
  • patch branches

Different people have different ideas about what sort of rules should govern commits, and in any case the rules for a given organization should be spelled out in the organization's SDLC. Still, it seem pretty clear that the rules should be increasingly restrictive as you move from the top of the list to the bottom. So, for example, developers might not be allowed to commit anything at all to types below the main development branch on the list above. Certainly, anything committed to the main development branch should not just compile, but work (i.e. pass all tests).

But what about developer and task branches? It seems obvious that code committed to a task-specific branch shouldn't need to pass all tests. The whole point of a task-specific branch is to provide version control while the task is being completed; if you could only check in "working" code, you might only commit to a task-specific branch once, when you'd finished the task. So, you might make a rule that code committed to task branches should compile, but doesn't necessarily need to pass all tests.

Developer-specific branches should be even less restrictive. Developer branches should be like private work areas where you can try things out. Being able to commit code to a developer branch gives developers the freedom to try things without worrying about losing work. It eliminates most of the risk associated with trying out an idea without knowing ahead of time that it'll work.

In some articles people even write descriptions how to create svn or git hooks that compile and test code before commit.

Again, such hooks can be useful for monitoring commits to the main development branch, production branch, etc., but should not be applied to developer branches. A hook like that could be used to automatically indicate whether a given task passes all tests, but it shouldn't prevent a commit.

7

Ask yourself what would happen if you committed halfway through a large refactoring effort, then got hit by a bus. What would the other people on your team do? You might be interested in Git; with Git, you can make commits locally, then push those commits in bulk to the server. Even if your server doesn't use Git, you can use it to stage commits locally before pushing to something like CVS or SVN.

8
  • 3
    I would never make big not staible commit. I prefer to make always small commits so i can always find out in history what happend. And so on Jeff, ask yourself what would happend if you commit hundreds of modified files with hundreds new and changed lines of code in one commit? No one except you would not understand what happend. Maybe it is better to make commits while developing?
    – Astronavigator
    Nov 14, 2011 at 11:20
  • Sounds like branching and then merging is more of a solution for you then.
    – Jeff Kelley
    Nov 14, 2011 at 11:23
  • it is just what i am saying: stable commits only for selected brunches.
    – Astronavigator
    Nov 14, 2011 at 11:47
  • +1 for git but -2 for the idea of committing anything until the end of a big refactor. Regardless of what CSV is used branching off a feature branch and commit often there is just common sense. That said, Git will allow this locally without all the big hassle of getting a new branch in the corporate-tight-ass-controlled-paranoid-nobody-touch-this TFS (or svn, or vss or timestamped zipped folders or whatever)
    – Newtopian
    Nov 15, 2011 at 5:36
  • @Newtopian, I wasn’t advocating for that idea, merely giving an example of something that would be bad to do. Nov 15, 2011 at 21:34
6

Let's look at it this way.

You take a week off work and when you come back you get the latest code from the version control repository.

You make a few changes that take about an hour and then set about testing it. The code initially doesn't compile so you spend a day working on removing errors in code that you haven't even touched, so that it will compile.

You then find that it doesn't pass any of the test routines set up for your project and so you spend another day correcting that and ensuring that all tests pass.

You commit the code and find that there are many conflicts in the code that you have written against what is on the repository. You spend another day resolving the conflicts only to find that it doesn't pass any of the test routines that it did a day ago.

That strikes me a a massive waste of time. In my experience, non-compilable code is worthless, and so shouldn't be committed.

In fact I would say that on projects that have automated test routines, code that doesn't pass the tests shouldn't be committed either.

edit

I see that you've edited your question in light of all of these answers.

You never stated that you were working on a separate branch (which in itself can lead to trouble if merges are too few and far between), but I would still stand by the fact that non-compilable code shouldn't be committed.

Even if your code doesn't work, at least if it compiles people can merge it, build it, test it and prove that it doesn't work.

What's more I don't honestly believe that you make many hundreds of changes to your code and never compile it.

Obviously there are many strategies of software development and some work better than others. From my experience automated testing (and even test driven development) is a necessity, even more so as projects become larger. The beauty is that everyone knows that the code works before it is committed, whether to a branch or to the main trunk. There are many tools available that allow for automated builds and testing e.g. cruise control.

But your original question wasn't asking about SDLC's or strategies or even whether committed code should work but only whether non-compileable code should be committed.

2
  • Why cant i just take latest stable code from tag or selected brunch? Why am i interested to get just latest revision? Look, i did not say that i would commit not compilable code to trunk (or create tag from it).
    – Astronavigator
    Nov 14, 2011 at 11:31
  • If you are compiling with Eclipse, it's perfectly reasonable to make hundreds of changes to one file when another doesn't compile. You may want to make a bunch of changes to one part of the project, making commits in your separate/local branch as you go, and (for the moment) not caring that it breaks other parts of the project. Aug 10, 2015 at 21:54
4

You appear to be talking about two distinct things:

  • Code must be compilable.
  • Code must work.

For the first, I would say that all code committed should indeed be compilable before you commit. This is also fairly straightforward to do as I would imagine you would be compiling your code as you make changes. Some IDEs (eg. Eclipse) will even compile as you save. If you are doing active development, compiling your code should be part of your workflow so this requirement should be trivial.

The benefit of this is that anyone can jump in at any point in time and check out the head of your branch and get started on further work without having to first sort out compilation failures. That person may indeed be you, after a break.

The second is a more complex matter and has more flexibility as "working" is a very subjective thing. If you are creating a new feature, then often it won't actually "work" for quite a while. I would suggest looking at it in a slightly different way: namely ensuring you don't commit code that breaks anything else. Breakages are easier to measure.

As above, this is helpful for anyone (including yourself) being able to jump straight into the branch and having non-broken code. Having said that, I often tell junior colleagues that they can go ahead and treat their personal dev branches as their own domain and commit broken (but still compilable) code if they find it useful.

Regarding your comment about committing many files and being unable to work out what has changed, the way I work is that I will commit code in a single "logical unit". Whether that be one change in a single file, or a refactoring across many files. Working in this way I know exactly what has changed per commit.

A common theme above is how your commits affect other people and their ability to work. Software development is a collaborative effort and it's important to think about and understand how the state of your code affects your colleagues.

4

Commit broken code? Yes. Push broken code? No. You should commit early and often in your own private branch -- That's what the tool is for, to track changes. When the code is working and tested, then you can push to others.

Of course, this advice only works with an SCM tool that provides both a commit and push/pull workflow. If you use a centralized system you absolutely should not commit code to the centralized server unless it at the very least compiles and passes a simple regression.

If your tool doesn't make it trivial to create your own private branches, pick a better tool if possible. Just don't go pushing/committing code that others can use unless you are reasonably certain it causes no regressions.

4
  • And if you can't use better tool on organizational level, git allows to export ( stackoverflow.com/q/160608/462370 ) files to other directory (so you need to remove all files, export, use the other tool to commit changes). Sure, it makes the work flow more complex, but the ability to easily re-work your patches in git is definitely worth it. Dec 15, 2012 at 17:32
  • "That's what the tool is for, to track changes.": Even locally, why would you want to track changes that break the code?
    – Giorgio
    Mar 12, 2013 at 20:44
  • 1
    @Giorgio: I can think of several reasons to track changes that break the code. Maybe the change itself is complete (say, a complete refactoring of function A), but the system as a whole needs a refactoring of B and C to work. I'll finish the work on A, commit, finish the work on B, commit, finish the work on C, commit, then do integration testing, and finally a push. Often I'll consolidate those local changes into a single changeset before pushing. Personally I find that having distinct atomic commits locally helps me organize my work. Mar 12, 2013 at 21:57
  • @Bryan Oakley: I understand. Each commit identifies a work package.
    – Giorgio
    Mar 12, 2013 at 23:03
1

In order to determine whether the code works or not you need automated tests. If you don't have them, having the code compiling is a minimum to submit to the main branch. In your private branch I guess you can have code that doesn't compile, but probably it is not a good idea; the reason to submit often is to do merges and to avoid losing data in case of disk failure. If you are really so afraid of losing all the work that still not compiles, it means that you are going to have heck of a job to get it back to a working state, not really a nice idea. If you want to do a merge, then probably you should tweak your branching model, or try working with less dependencies.

-1

I do not agree with the principle: "Commit often (every evening) so you don't lose your work." A version control system is there to store meaningful revisions of your code, not as a backup facility. You can use backups (zip / copy / rsync) to save all your current changes.

I think you should only commit code that compiles. For this: update / merge the current state of the repository, compile locally, commit. The time you spend removing conflicts locally is time that otherwise will be spent by several members of your team when they retrieve your changes. Do not force others to fix your errors, if possible.

You should also try to commit complete features, as it is more difficult to track changes that have been committed at different stages. If you are working on very complex features, consider working in a branch and then merging everything to the main branch in one step.

4
  • 1
    if commiting every evening is often for you, how often do you commit Giorgio? Nov 21, 2011 at 13:08
  • A small bug fix can require half an hour so I can have several of these commits a day. A major feature can require one week or more and I commit only when I have implemented all the required functionality. Committing small pieces only makes reviewing more difficult.
    – Giorgio
    Nov 21, 2011 at 15:51
  • @Astronavigator: IMO it is not a matter of how often, but of committing meaningful revisions instead of committing just the current state of my source code because it is 5 'o clock in the evening and I am about to go home.
    – Giorgio
    Dec 7, 2011 at 19:59
  • I respect the opinion of users who downvoted this answer, but I would like to know this opinion. Especially, I would like to understand what is so insane about fixing problems locally instead of spreading them throughout the team (I spoke about SVN commits / git push, not about local git commits).
    – Giorgio
    Mar 13, 2013 at 6:44

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