7

Whenever I do a check-in, I always check in from the root of the project... i.e. check in all the files in my working copy, so after the check-in the source control repo contains exactly the same set of files that I just finished testing in my local copy. I also make sure my source control is set to flag local files that are not under source control. In general, there are none of these files... if there are, I either add them to source control or mark them as "ignored". I also check in all my changes together in one check-in.

A lot of colleagues check in much differently. They carefully select each file to check in, as if they are a master jeweler selecting only the very best gemstones to set into the royal crown, and they check in each one as a separate check-in. They rely only on their memory to figure out which files need to be checked in, or especially added to source control.

The results are quite predictable... frequent broken builds because they forget to add their new files to source control or forget to check in a changed file (especially changed project files).

I have mentioned this to them and they never seem to change. When I mentioned it to the team lead he said, "this is just a different way of working". To which I may respond: What if I want to drive my car with my eyes closed? Is that just "a different way of driving"?

Am I right in being bothered by this practice?

6 Answers 6

11

Don't be bothered by the practice of checking in individual files - if someone can do that and make it work, that's fine.

Do be bothered by people checking in broken builds. The primary concern is the result. Addressing the root cause is absolutely a good idea, but the best first step is to find the appropriate motivation to stop checking in broken code.

1
  • 2
    Checking in a broken build is sloppy, and it shows a complacent attitude toward the project and its success, AND the time and work of the other people on the project. Occasionally checking in and missing a file is understandable but if it occurs more than once or twice there's a problem with attitude. Nov 1, 2010 at 0:25
4

If broken builds are a problem, I would suggest you evangelize automated builds for your team. If someone checks in a file that breaks the build, an automated build will tell you straight away, rather than waiting for the next guy to come along and find out, spending 15 minutes tracking down the person who's check-in broke the build (because it might not have been the last check-in!) and so on...

If people are checking in files one-at-a-time, you can delay the automated build by a few minutes after it detects a check-in, to allow people to check-in the rest of their files. This is cause for concern as well (question: do they put proper comments in the commit message?), but it's not as big a problem as breaking the build is.

3
  • We have automated builds... it doesn't help... some people ignore it or think it's not caused by them
    – JoelFan
    Nov 1, 2010 at 12:22
  • @SpashHit: wow, I feel for you :-) Nov 1, 2010 at 22:38
  • @SpashHit A good CI framework will know who made the commit that broke the build and can email them directly letting them know
    – Alb
    Jan 30, 2011 at 15:03
3

Yes, I think you are right.

At best (if they're always doing it correctly) what they're doing is just a slower way of doing what you're doing.

The bigger problem is that it's exposing less than ideal working practices. Firstly it looks like they're working on more than one thing at a time - so rather than working in small commitable steps they're doing some big change that's uncommitable and then have to do some other task in the middle and commit that. Or maybe they're not cleaning up after themselves and their uncommitted local changes have all sorts of junk in them (big blocks of commented out code, redundant methods, broken stuff that was just a temporary experiment).

I've worked with people who do this and it's always a nightmare - the worst situation is when they have uncommitted junk locally then when they update their local version gets borked, but some of the stuff they want to keep is in the same files as the junk. As a result they just get lost in sorting out stuff they want to keep from stuff they want to revert.

2

Change lists are an awesome concept. Only a set of coherent changes (which solves one thing) should be part of a single change list which is checked in together.

To add to my answer: Ideally you should be able to build a change lists of your files, pass it over to a clean enlistment (which is totally synced with the repo) apply the change list and build it before checking in. If your team mates aren't bothered to do it make it as part of the code review process.

1

Leaving the orthogonal problem of how to not forget to add files aside, your approach can lead to another problem: accidentally checking in changes that were just for testing etc.

Handpicking the source files (or sub-directories) to commit is one possible approach which can be done responsibly and without regularly breaking builds (utilizing the status info of your VCS). The real problem here is spreading the changeset over many different check-ins, one for every file. This will inevitably to lead to difficulties when the version history is needed for anything.

1

It's possible to work a little sloppy if you have good tools to keep bad builds from happening. Generally this will keep the team happier than requiring constant vigilance.

Setting up a continuous build is the first step. If broken builds are a problem, it will reveal it.

Then you could write a submit script that, given a list of files, checks out the project head in a new directory, patches them in, runs tests, and checks in the files together if they pass. Then they can cherry-pick if they like, so long as they use the submit script.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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