views:

131

answers:

5

Yesterday when i checked out the latest version of our internal tool i saw about 30+ new versions. This got me curious since i thought that somebody finally fixed those annoying bugs and added that feature i was waiting for so long... and guess what? None of this happened, someone just thought it would be nice to update some headers and do a minor adjustment of two or three functions. Everything in a separate commit. Great.

This raised a discussion in our team - should this be considered ok, or should we prohibit such "abuse"? Arguably this really could fit in one or two commits, but 30 seems to much. How should this be handled - what is the best practice?

+6  A: 

You should be committing any time you make a change and are about to move on to the next one.

You shouldn't commit anything that stops the project from building.

You should be filling on the commit message so people know what changes have been made.

That'll do for me.. I don't assume something has been done unless I see it in the commit message...

Shahin
Don't commandments traditionally start with "Thou shalt..."? *SCNR* +1
sum1stolemyname
@sum1stolemyname: never read an RFC?
Konerak
@Konerak: SHOULDN'T RFCs traditionally start with ["You SHOULD..."](http://tools.ietf.org/html/rfc2119)? :p
KennyTM
http://www.ietf.org/rfc/rfc2119.txt
Konerak
Maybe I should edit it to make it sound more friendly...
Shahin
+2  A: 

I wouldn't care about the number of commits as each commit keeps project consistency (build will still succeed). This is some internal count that shouldn't bother you. If you want to change something here, better tell people to use some structured commit messages (like "[bugfix]...", "[feature]...", "[minorfix]").

By the way, if you want to know if bugs have been fixed or features have been added, using a bug tracing system is much better than checking commits in a SVN-like tool.

schnaader
+4  A: 

Generally I think a commit should relate to one logical task, e.g. fixing bug #103 or adding a new print function. This could be one file or several, that way you can see all changes made for a particular task. It is also easier to roll back the change if necessary.

If each file is checked in one by one, it is not easy to see the changes made for a particular update / task.

Also if multiple tasks are completed in one commit, it is not easy to see what changes belong to which task.

Neal Donnan
+1 Exactly, i think that a commit should reflect a logical change (bugfix, feature, refactoring...), not relate physically to a file or function.
PeterK
What is making the change requires 2 weeks of work. Surely you shouldn't only check in after 2 weeks but every time you have something that doesn't break.
Johann Strydom
@Johann Strydom: true, if someone is doing a big change, he should divide it into a couple of smaller ones. Anyway i still think this can be done in a way where everyone can understand from the commit messages what is being done and why.
PeterK
@Johann: yes, you need to check in as often as you can, but a 2 week task has multiple smaller tasks that make up the task and you would check in logical smaller tasks as you go. As long as you do a piece of work and check it in with everything that makes up the change in one checkin. It just makes it easier to see what's happening in the code base. I often read the check in comments of what my team members have done to see what changes are happening.
Neal Donnan
+1  A: 

The battle against code entropy is an ongoing team effort. Minor checkins where one just 'fixes broken windows' along ones way should be encouraged, not frowned upon. The source repository is the wrong tool for keeping track of bugfixes - that's what a bug tracker is for - so the inconvenience in locating fixes when scanning the code repository and not the bug repository seems utterly negligible to me.

I work in a moderate size team on a large code base (~1M LOC) with a huge history (~20Y). A lot of the code is a pile of mess - rotten branch logic, deprecated API, naming conventions, even random indentation often makes it a misery to read. I started a habit of minor "drive-by" readability improvements, to try and fight complete code rot, and am trying hard to get teammates to adopt the same habit.

Unless your circumstances are radically different, I would try and think favorably on any such initiative. The alternative (which I'm familiar with all to well) is fearful stagnation, which dooms any code to rot.

Ofek Shilon
It's not about stopping people doing things, it's just about saying it would be nice if when someone made a fix that they check in the changes to the files they fixed in one commit so you can see everything that was changed. It could only be one line in one file with the checking saying "Fixed spelling error in dialog box" that took 5 mins. I usually look at the changes that have been committed when I do an update to keep up to date with what is happening in the code. Seeing each file checked in individually with "Did some stuff" doesn't add much value.
Neal Donnan
A: 

With some source control solutions, like TFS and SCM Anywhere, you can shelve the changes if you are not ready to check in it. Check-in comments should help too.

Catherine