Is it good idea to require to commit only working code? [duplicate]

Question

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?

Answer 1

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.

Answer 2

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

Answer 3

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.

Answer 4

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.

Answer 5

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.

Answer 6

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.

Answer 7

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.

Answer 8

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.

Answer 9

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.