Every commit a mini-release
Last week I opened a ticket for a bug in the development branch of CakePHP and it got closed as “invalid”. The same happened with tickets from others, too. Later PhpNut wrote in a comment:
[...] the tickets that I closed where invalid because the code was not completed.
Well, I think this approach — committing incomplete and failing stuff — is not optimal, as you no longer know whether an issue (e.g. a fatal error) is a bug or incomplete code (i.e. the developer is aware that it fails).
Hence I think it makes sense to treat every commit as a mini-release, i.e. you always want to deliver a running version to those using the code from the repository. Sure, that’s not always possible, as people make mistakes, but it should be the goal.
The easiest way to accomplish this goal is to have tests: only commit your code if the tests are green. If there are no tests (or not everything is tested) then it helps to move forward with small steps. Make a change, verify it works, commit it. Then repeat these steps.
A nice side effect of such an approach is that it is easier to understand your commits. Instead of one big commit adding for example ten features you will have ten commits adding one feature each.
As you see, one small change in the way you work may bring you quite some benefits :)
aren’t you supposed to use branches of svn when developping something that would take a little time ?
You have some good points but not always are you able to commit code that is 100% complete or passes all the tests. When working in team, sometimes we still have to commit our work when we know (and the team knows) it is incomplete. We even should not defer a commit for too long. We want to introduce our new changes / additions to the rest of the team soon so others can checkout the code to work on. Prompt commits also minimize too many unnecessary conflicts.
@speedyop: Thanks for your comment!
Even if you use branches you still have the situation that there may be others (e.g. co-workers) using the same branch and who may be affected if you commit incomplete and failing stuff.
And at least for me a commit is a sign that something has been finished, e.g. a bug was fixed, or a refactoring was done.
I have to agree with Zonium on this one, svn purpose is also to share partial code if the rest of the team needs to use your functionalities to complete a working build.
That’s why a commit is not a “release” but simply a sign that the work you were supposed to complete on a certain matter is done.
It’s not unusual to commit a finished part of code that affects other functionalities manages by teammates who then check out your work and integrates it in their part. Imho.
@Zonium/Repsah:
I think what you’re saying is correct… for a closed source project.
When working on an open source project however there needs to be stricter guidelines for what should and should not be commited. For example, if a commit knowingly breaks something (or is not complete) then simply adding a commit comment stating that would be a good idea. At the same time though it should only be committed in the first place if the code is absolutely essential to another devs current work.
That way people using the code won’t waste their time adding worthless tickets pointing out known faults, which are subsequently ignored.
@Ben
You surely have a point there, but still there should be a way to distribute your code to other developers if their effort is needed for the whole to work fine. Personally I’m not too keen on using means other than the official one, which, in this case, is the svn, but you surely have a valid objection saying that open source is a different matter.
I believe that many of you are missing one hugely important factor… Downloading and using any source that is not listed as “stable” is your own decision, and should not be expected to always work 100% of the time (which is why Dan and others posted bug reports that were understandably closed).
The thing here, is that many teams work in many different ways. To expect one team to always work like another is irresponsible. Certain teams work better one way, others another. If there were two branches - a developer and public, perhaps it would be helpful, but then…you’d need to commit time and resources to keeping the public branch updated - time that could otherwise be spent on the code itself.
I think in the case of CakePHP, the way they’re going about it is probably the fastest and best possible solution. If you want to be bleeding edge with the source, you’ll have to deal with a few (or more) stumbling roadblocks. :)
It’s my understanding that’s what branches was *made for*.
Seems like you’re grasping at straws here… ?
This is the blog entry I’ve been using to guide my source control efforts for the past year.
http://netevil.org/blog/2006/dec/coding-for-source-control
I’m also working similar to your approach when working with multiple developers. I usually commit when a specific ticket is completed and works. It can be frustrating for your colleagues if their running app is broken when they update your partitial completed code..
When I’m the only one working on a project it doesn’t really matter.
With our projects we use the following SVN setup because of the growing # of partial/incomplete commits.
We have both a live site and a live-dev site. The live is checkout’ed to the trunk and the live-dev is a checkout of the ./branches/dev (www.site.com and dev.site.com respectively).
Obviously committing partial or incomplete code to the dev branch could render our live-dev site useless. So on each of our local devs we use the “private branch”. Our flow goes:
Private Branch -> Public Branch -> Public Trunk
Only the devs have access to RW on the private branch. We add specific keywords to commit messages so the other developers always know what they are and which revisions need merged:
- Critical / Bug Fix (almost always immediately merged)
- Enhancement
- Partial (never merged by itself)
- Complete Partial [partial revs list]
- etc..
After they are in the public branch for 1 week then they are set to be merged to the public trunk. (Each commit goes through a 1 week “test” period)
FYI, don’t try to manage SVN merges by yourself. There are many tools out there (svnmerge, etc) that help with managing these.
@John: If you were commenting to me, I don’t see how that’s grasping at straws. An axe was designed for chopping wood, but if someone uses it to chop up ice, is it necessarily wrong? I was commenting over how different people/teams/groups use similar tools differently to achieve the same goal.
@Brandon: No, the comment was directed at the author. It seems like this post tries to create criticism out of nothing.
Essentially, it boils down to “it wasn’t done my way.”
@all: Thanks for your comments!
@Zonium, Repsah: I think I was a bit imprecise with the term “incomplete” as there are two different cases. On the one hand there are the “good” cases, e.g. committing a form together with an empty action. Even though this feature is incomplete, it doesn’t break anything. On the other hand there are the “bad” cases which break stuff, e.g. renaming a method and committing this change, without adapting and committing the code which uses this method.
But if there are tests around, then I think they should pass before you commit your code as it is annoying to have a red bar ;-) And it shouldn’t take you long to make them pass if you move forward with small steps anyway.
I agree with you that you shouldn’t defer a commit for too long (I experienced the negative effect of this more than once myself *g*). I think the motto “release early, release often” also applies for commits.
@Ben: Thanks, you bring it on the point :)
@Brendon: Sure, it is my own decision to use the latest code and I am aware that I am using an unstable version. But “unstable” means for me that things may change and are not yet finalized, but not that it is buggy by intention.
I agree with you that every team/person has its own way to work, and what works for some, may not work for others. That’s ok. On the other hand nobody is perfect, so there is always room to become better. This article may be an inspiration for you to question your own way of committing stuff. I don’t expect everyone will now start using this approach ;-)
@Jeff: Thanks for sharing this link!
@Eelco: Well, I think it makes sense to follow the same approach when working alone so that it becomes a habit.
@brandon: That makes sense. A similar approach was used in a company I worked in the past. There were also three stages (dev, staging, live), but only one branch was used for the dev site. If you wanted to put something to the staging server you simply exported the code from the dev branch. And to go live the code was copied from the staging server. This worked fine at that time and you didn’t have to care about merging ;-)
@John: Well, this article is meant as feedback on how the commit process could be improved. And yes, it is written from my perspective: I described what I think the problem is, and I described what I think could be done to avoid it. Now it is up to you what you do with it ;-)
I suppose i just disagree with it. It doesn’t make much sense to keep really strict working policies on development branches. That’s pretty much the entire point of a branch.
If you’re talking about trunk, I’m with you. Otherwise, I don’t really see your point.
@John: What I wrote here is about committing code in general, and I think the way you work should be the same whether you deal with a development branch or the trunk. So if it is not ok to commit buggy code to the trunk, why should it then be ok if you do the same on the development branch?
And I don’t see it as a really strict working policy if you say: “Only commit stuff that works”.