Ways to handle a programmer who is not thorough.

Question

I'm working in a startup company where the development team is a small team of three programmers; A project manager, myself and another guy.

The issue is that the other guy regularly commits code into SVN that doesn't actually work! The base usually compiles okay, but the references to views/stored procedures in SQL either don't work or he forgets to commit those files, making it a nightmare trying to get the latest build working.

Today for example, I had to go to a client to perform an upgrade. Everything went okay (database scripts and client update) but it was soon discovered that one of the reports didn't work; A report which was a specific request from that client and one of the milestones of the upgrade. It was working when the upgrade was scheduled but obviously at some point he committed some code which broke it, without testing it himself.

Due to time constraints and other factors there was obviously not enough testing done before the roll out but it simply wasn't possible in this instance. It was frustrating to the nth degree because I look like a flippin' idiot in front of the client for something that wasn't my fault.

How would you handle a situation like this? What steps would you take to help improve the situation in the long run, short of firing the person in question?

Things to take into consideration:

• Money is very tight
• He contributes to other facets of the business, not just development

Answer 1

After checking the source code, tell him to get the souce code from SVN and test it. Even he can run the Unit test cases on the newly taken code if there are any.

When I have started my career, I was doing like this to make sure every thing is in.

Answer 2

Are they resistant to change, or just careless? The latter is much easier to fix than the former. Consistently bringing the problem to their attention as soon as possible after the event, along with some mentoring in how to fix and avoid the problem, will probably work wonders. Run the test suite before commit, all that sort of thing.

If they're just a curmudgeon, then you might need to perform more extreme measures... "automated test suite must pass before commit is accepted", or not allowing their code to hit the golden branch automatically (must pass review). That'll be a big resource drain on such a small team, and you need to weigh the benefits of his code contributions against the cost of either (a) letting the problem continue (and take the reputation and post-relesae bugfixing hit), or (b) cutting him off from the code, and getting him to focus his energies elsewhere in the company.

Answer 3

You're not the boss, right? So you can't force him to do what he clearly needs to do. But now you do know that you need to do your own testing, and not rely on his stuff working.

This time you couldn't test due to time constraints; but in the future you'll have to build this kind of testing time into the schedule: Block out an hour, or two, or even a whole day or two to make sure you'll have a working build to take with you. You'll have to include time to fix bugs, of course, not just to test. (Or you might be able to get earlier versions to find a working one.)

If your boss asks what you're taking this time to do, you can tell him exactly what happened last time. Otherwise, tread cautiously, because trying to get someone else in trouble, fired, etc. always looks bad no matter how justified it is.

Answer 4

When you have code that has been tested and judged releasable create a tag. Then if you need to deploy check out the tagged revision.

Do daily builds and make who ever breaks the build babysit and fix the code until someone else breaks the build.

Answer 5

Setup a continuous integration server of some sorts. The flashing red light that shows up when he breaks the build will at least show him that he's done something wrong. Have you approached him about this? Perhaps he doesn't even know he's breaking the project. Perhaps a nudge in the right direction is a much better action than being mean.

Answer 6

This may just be a case of old bad habits.

How about implementing a continuous integration process (CruiseControl?) that builds and run some necessary tests (unit or functional) every time artifacts are checked in into SVN?

Everyone in the project will get immediate notifications if the build fails. This will at least reduce, if not completely eliminate, cases of bad check-ins by the developer.

Answer 7

Something that can be done to mitigate the damage - especially for delivering special releases/demos to customers:

If you're going to be delivering code to a customer (a particular patch or whatever), that work should be done in a separate SVN branch. That way the changes made to what's being delivered to the customer can be restricted to only the changes required/requested.

It should not get every change that every developer checks in every day.

This way if you're constrained in your testing resources & time, you only need to test the few things that have been changed. And if someone working on something unrelated to the immediate deliverable checks in something boneheaded, it won't be in the branch that you're demoing to the customer.

Answer 8

In addition to the other good ideas involving continuous integration, etc...

Give him his own branch of the code to work in, so he can check in freely. You can bring his changes into the deliverable branch on your schedule, so the deliverable won't accidentally have his bad code.

Answer 9

We used to have this policy at a place where I worked:

First checkin that breaks the code:

You get the JarJar of shame. This is a small figurine that sits on your desktop and taunts you with your inability to do anything.

Second checkin that breaks the code:

Still the JarJar, but should have been a mask or dunce cap. Really, don't break the damn build.

Third checkin:

Serious performance review. This person is on thin ice.

Fourth checkin:

You're fired.

Offenses decayed after a week. If someone got to step 1, and then step 2, they could work it off in two weeks, but would decay down to step 1 after a week. Five bad checkins in a row, but all before someone else noticed, was just one checkin.

Of course, your variations on the rules can be adapted, but seriously. Having JarJar sit on your desk, taunting you, should be bad enough.

Answer 10

Hi,

1. Don't let him code? Make him a part of another section of your company since he also does not development work.

2. Don't let him submit? Discuss the changes he has introduced in meetings before you merge.. this way I think he will get the hint and become more aware of his shortcomings.. and you also can blame yourself if you failed to notice something wrong with his changes.

3. You could use Git for version control .. I only say this because it has made it easier on me to handle code that is merged between very different configuration files where not even I can remember well how to maintain the different setups. Branching, merging, conflict resolution and rollback are IMHO easier than other source control environments and you can easily see what's different.

link to SlideShow Explaining my point from Scott Chacon

Answer 11

Here are some drastic measures you can take:

• Create an SVN rule that requires a comment and associated ticket is entered in the commit message, don't allow commits that do not follow this
• Have a daily team meeting discussing what everyone is working on. (that way you will have less surprises)
• Give him a project that feels important to him but has only non-critical impact on the project.
• Start a continuous integration server. Notify everyone immediately of broken builds.

Answer 12

Daily builds with emails sent to all development help.

A big indicator of some sort is a nice simple way to declare people inept. We have a newly created 'award' appropriately named 'Noob of the Week'. Its an old hard drive with big writing on it that declare 'Noob of the week'. Its placed on the persons desk and they have to fess up whenever prompted why they have the award. Also when its awarded we usually have 3 or more developers stop by when its handed to apply more embarrassment.

If not just for fun/morale, it does seem to stay on some desks more often than others.

Edit: I think my post is being interpreted harsher than it really is. We generally pass it around when someone does something that an individual new to the programming language would do or was after a second look redundant. For example assuming string.Replace modifies the current string instance, forgetting the case keyword on switch statements.

Answer 13

Hold his hand for a while and ensure that everytime he is about to check in some code, that you want to personally review the code and the piece of work he changed. He will feel very uncomfortable and possibly even incompetent. This will hopefully make him realise that he has to be more thorough as noone wants to have a senior looking over their shoulder all the time. If he still cant get it right, then perhaps he is not cut out for programming and perhaps better utilised somewhere else in the business.

Answer 14

Did you and him have a discussion about what should and shouldn't be checked in? I have seen both schools of thought on how to use source control where one side is that whatever is committed has to be this flawless, releaseable code, and the other where anything can be checked in since all it does it keep track of changes, they don't actually have to work.

I'd have a chat about how to use SVN here at company ABC and that this is how we should use it here. Let him ask questions and be prepared with some basic ground rules about what you'd expect and that the project manager mayy be called in to handle disputes in the future if problems like this keep popping up.

As for the issue with the upgrade, was there time spent to make sure that everything was ready and good to go? This would be a key point for myself and I would probably take the half day before going to the client to do some dry runs and see that everything worked OK. I have been in a few companies where there would be times that we'd have rules about noone touch production while something was being done.

Answer 15

Use ScumBug.

SCMBUG

It will let you set Source code policies.

And then we add automatic builds using something like HUDSON. (continuous integration)

Continuous integration combined with immediate refusal to commit if it's inappropriate.

What you do then is use the last build Hudson tagged as okay, and not the head.

None of this costs ANY MONEY to set up.

And it makes the RIGHT THING the easy thing. It stops eve people like you or I, who would never skip anything stick to the right path.

The "you broke the build" data can then be used to fire or transfer the guy in due course if he doesn't shape up.

I used to automate away processes in manufacturing and we improved productivity and customer satisfaction. Automating Software Engineering processes can make process compliance automatic, which makes some kinds of audits easy to pass.

Answer 16

In addition to what everyone else is saying, consider this: introduce a code review process. All commits should be signed off by at least one other team member who can do sanity checks for correctness, test coverage, style and anything else you care about. Time spend reviewing code can often save a whole lot of debugging time down the road. Unreviewed commits should be detected (or ideally, prevented) and explanations demanded.

Answer 17

Look, if I've spent a day working on some new feature, but it happens to break the build, I'm checking in. I'm not going to risk losing a day of work just because we haven't planned for some features being bigger than an hour of work.

What I will do, is plan for the current state of SVN to be broken at times. As someone else pointed out, use tags, and trunk correctly. You could even set up other branches, such as "stable", "unstable", "broken" etc... So that I can save my work, and you can not look like an idiot. Everyone wins.

Set up SVN checkin proceedures, so everyone knows to branch when checking in broken code. Make sure everyone follows the proceedures. If you don't have the authority to enforce this, take a proposal to someone who does.

What's the point of using SVN if the only thing you ever do is commit, and check out HEAD? If that's how you're working, you could just as well be using a standard network drive. If checking in a broken build is a disaster, that looks like a process problem to me, not a programmer problem.

Let's just drop this punitive crap, okay? We're all human, and we're all learning. I'm interested in solutions, not new problems.

Answer 18

Consider pair programming, for part or all of the day, but at least a couple hours a day. As you go through the process, he gets an opportunity to walk through your process with you. Your process has been working for you. He'll get a chance to see how you think when you write your tests and write your code. He'll see what you do to make sure everything's working before committing to revision control. Furthermore, you talk through things as you do it, think together, and he'll be infected with your good habits. He'll get hands-on-keyboard as you walk him through it.

This way, you can communicate here's what's been working for us, instead of being in the position of pointing the finger at him.

Also, follow the many good suggestions from other folks here, particularly with regard to Continuous Integration.

And, Godspeed, my friend. :)

Answer 19

I think I'm well-qualified to answer this question because I am not thorough. I am good at thinking about and writing code, but I'm not good at being careful and "dotting the letters that need dots".

Here are the things that help me:

Remove time pressure As long as I think I'm in a hurry, I'll take shortcuts. Deadlines, the end of the day, threats to cut features that aren't done in time, people banging on my door and nagging me. This kind of pressure brings out the worst in me. Ironically, the best fix is to have MORE deadlines! Make the steps smaller, so that I only have to think about a little work instead of a lot. I need my coworkers to remind me that it's OK to stop and come back tomorrow; that it's OK to let something go because we're out of time.

Remove feature pressure. As long as I feel like there's too much work, I'll again get sloppy. Again, the trick is to break things down in to smaller pieces.

In my last job, we were doing 1-month releases (which was a great improvement over multi-year before that!) but I would have done better with 2-week releases, and 1-week iterations.

Automated testing Fast, comprehensive, automated tests mean that I can check my work before a commit without a lot of effort. When the normal path is quick and easy, shortcuts become less attractive.

Automated commit validation aka Continuous integration. In one job, every set of pending changes went through an automated system that would build, deploy, test, and commit changes, mark bugs in the database as "fixed", and email the team. It is really hard to take shortcuts when a computer is doing the work for me! Making this really reliable is critical; making it fast is important, too.

Small steps Making my changes small stops me from getting overwhelmed by complexity, or from forgetting things I need to do before committing.

Branching Using a branch to explore changes in small steps can be helpful, although it adds a new kind of complexity (you have to think about branches) and you have to be careful not to wait long before pushing your changes up to the parent branch.

Pairing Having another programmer sitting next to me helps me stay focused on the big picture; helps me remember to keep all of the above working for me; helps make programming more fun.

Answer 20

You could put him on his own branch. Branches are cheap and svn v1.5 has usable two-way merging.

Answer 21

To me, the problem seems like you went out to a client with improperly tested code. Every developer in the world occasionally gets it wrong; we are all liable to make mistakes, that's normal. If you don't want to be embarrassed in front of clients, don't go there with untested functionality.

After you've fixed your testing/release procedures, you probably want to have a talk with the programmer to discuss his role on the project. Way back I had a similar problem with a younger programmer, so I built an automated script to list out all of the changes. Before each release, I'd do a quick review and flag anything questionable. That little bit of investigative work on several instances saved us from releasing a bad version, but didn't heavily interfere with anyone's ability to still commit good work.

If they're still playing fast and loose, instigate bug tracking for the releases with something like Trac. If 95% of the bugs are coming from one person, it becomes easier for management to deal with the issues.

Paul.

Answer 22

If he's a young, inexperienced programmer who needs mentoring you're in luck. Be his mentor.

In general the best way to deal with it is for him to experience the pain first-hand. Keep little distance and little delay between his actions and the results. Talking about it and suggesting improvement is good if you're a mentor but there's no substitute for being at the client's office, suffering the results of one's own mistakes and feeling stupid as you try and explain what happened. Believe me, it stings and the lesson sticks.

Confucious said:

I hear and I forget.
I see and I remember.
I do and I understand.

It's still valuable advice today.

Part of being a mentor is putting a person in the best environment where they can learn most effectively. Let them "do and understand" as much as you can appropriately, under the circumstances, allow. Then, when you post-mortem the result and offer advise they will understand your words and improve.

Answer 23

I am quite surprised that everyone thinks the guy who committed faulty code is responsible. The problem is that you use code that is not tested - if you have no time to test, that's where the fault is. Even with proper unit testing, that does not replace preparation of a demo. This is really a sign of bad code/release management: you should simply not demo code right from your development branch directly.

Firing someone because he is committing buggy commits is very stupid IMHO: what will happen at best is that people will not commit often, which defeats the whole point. You should use branches and tags to handle release.

Answer 24

It is partly your fault not to test thoroughly before going to the clients. If you find a bug during a 1hr demo in your clients, there is no excuse not finding them before you go theirs. You could have run the demo yourself if the demo didn't involve any critical data only available at your clients.

If you think your colleague is bad and you are superior, that is good on you. Don't complain others are not as good as you are. If they are better than you, you will be feeling comfortable and they will be the ones that write this article here. Bill Gates never expects many people smarter than him and he probably thinks that 95% of the guys in Microsoft are as good as him. But he still successes. If you are the best, it is 100% natural to find everyone else worse than you. The sooner and stronger you feel that way, the better you are.

One thing, not complain too much thing here. This wouldn't help your colleague to improve and wouldn't solve your problem. Tell your colleague his problem. If he refuses to listen, tell your leader. Setup tech meetings with you three and discuss some rules to improve checkins and code quality. I think everyone here won't deny that it is not easy to write beautiful and faultless code. So be patient and help others out! This is the way that lead you to the leadership.

Last word, if you are really brilliant, don't expect others to be as good as you. Help them to improve and finally you will realize that, even if your teammates are far behind you, your team is far better already to produce something brilliant.

Good luck.

Answer 25

You need more automatic testing to build and test of what is in your version control repository. Tag the repository when all the test pass and use this for clients. This would have gotten your fellow developer to fix the code sooner and you would not have gone to a client with untested code.

A repository of test build up over time shows bugs you never thought to test for when testing changes. I do not always fully test my bug fixes because of our testing setup I know that if there is a problem I will get a bug report.

For good software the testing effort should be equal to the development effort and the running of test should be as automatic as possible.

Answer 26

It's really simple. Here, let me break this to you so you can skip deep reading into the rest of the replies. (I was in the same situation as you before)

1. You need to test them before you deliver. How you test it is up t you. Whether you setup integration testing with daily build, or put aside 1-2 hours to test it out before you go to see your client -- it's irrelevant. You need to test your stuff to make sure it works before you send it off.

2. If you don't have much time, then come out with a quick checklist with the items to check before you go to your client. Update the checklist often and make sure everyone know where the checklist is. Since there's only 3 of you, assign yourself to go through the checklist. This is the part where you pretend to be the in house QA Manager. :-)

3. Use a bug tracker -- it's will be easier once you can look at the overview of the stuff that broke. It also helps you answer these vital questions:

   • What is broken
   • Why is it broke? (Ask this question 5 times if you a QA freak)
   • What can we do to prevent it from breaking in the future

   This helps to instill a mindset of problem solving and problem prevention.

4. Discipline is very important -- if you're not going/able to stick with the plan actions above then regardless you will always get the same problem.

It's that easy to get immediate result.

Go through these steps with everyone. Make sure they understand that this is the new plan to not screw up in front of customer again.

Answer 27

I work for a small startup as well. It is incredibly expensive for us to make a mistake and not catch it before it goes to a client's required testing house. For a long time, schedule pressures and inadequate testing was killing us. Coders would check in "last minute" code that would compile fine, but later show up as a crash, freeze or other errant behavior. We were lucky if we caught it in internal QA, but even that cost a lot of money to go back and re-fix something that should have worked.

What turned it all around for us were two simple things:

1. Change lists.
2. Demos.

Every time we do a release (but before sending to internal QA), we create a change list from version control (we use SVN as well). Then we all get together at some point and do demo of all of the changes we made. Did it actually fix the bug? Is the feature fully implemented? Did it break anything? We don't catch subtle things, but all the silly, "shoulda-seen-that" problems go away.

Mind you, this isn't a code review (which should have happened earlier in the cycle), it is a demonstration of well-behaved code; it is basic incremental development. By having an audience involved, I have found that two things have happened: 1) my coders are much more thorough in writing and testing their code (long before it gets to QA) and 2) they take much greater pride in their work. After all, it's very rewarding to successfully demonstrate code that works, no matter how small.

We always make room in the schedule for demos, no matter what. Sometimes its a big meeting, sometimes its small. But an extra hour or two a week to show off isn't the end of the world and everyone can feel confident that the client is getting what they want.

Answer 28

• Just back out things that don't work. A broken build is a problem for the whole team. Make a policy that if the build is broken for an hour, you just back out the change that broke it. If he has time to commit, he has time to wait a couple minutes to make sure the build doesn't break. Your top priority is the team and the project, not any one developer's new code, so your policies need to reflect that.

• Don't demo trunk. Demos for clients should be a known-good configuration. Trunk is latest-code, not known-good. Branch, tag, keep a working copy from last week, whatever.

• Get a DVCS. With SVN, the only way to really save your state is to commit it. Maybe if he had a local repository, he would commit all day long, and then push less often, with no fear of losing history.

Answer 29

Maybe he's just a NNPP. If yes , some good ideas here

Answer 30

NEVER Install a client base directly from your source control, for this very reason. Someone can break the client code inadvertently.

It sounds like you need to institute "stages" or multiple systems. At most places I've worked, we have three layers that code goes through before the client sees it.

1) Development. This is where code is checked in/out. Joe-Schmoe should be able to check code in/out here, and here only.

2) Testing/QA Code files (aside from HTM files) never make it here. When a "release to QA" is made, the person testing the code puts it here to do their testing. In the past two jobs, this area resided on the DEV server, but in alternate locations to save the cost of a separate QA server.

3) Production -- IN HOUSE. We also have an in-house version of a valid "production" instance. At one job, where each client had their own code, this area was constantly being wiped and re-created for different tests, validations, and releases. At my current job (the CEO is notoriously tight with money), I have only ONE serveer for all three instances, but I ahve them separated by directories.

The Production instance should show us what our clients should be seeing.

When you deliver a product to a client, you should take the code from your PRODUCTION instance, where it should have gone through a quick pass to make sure that this very incident DOESN'T exist. Even if Joe-Schmoe checked in bad code to development, it shouldn't be anywhere CLOSE to getting to your client on accident.

Hope that layout helps.

--Jerry.

Answer 31

First off, let me say that I've been plenty guilty of all of these things myself (and still am, despite my best efforts), so don't take this personally. Secondly, I tend to see everything as black and white, with very little grey between. I, contrary to most people, think this a good thing. It's fuzzy logic - it removes the endless, pointless focus on the trivial and gets back to the heart of the issue. If someone is desperate to focus on grey, I say "fine, the instant the black and white is sorted, we'll get right on to that", and that's last I hear on the subject. 99% of grey is just a convenient place to hide from facing up to the black and the white :)

I'm afraid that I believe, from your question, that trying to change this person's behaviour is just hiding from inconvenience. If you want things tested every time, make it a rule. If he doesn't follow the rules, discipline him. If you don't do this, you can't expect anything to change. You've tried the 'hope he takes the responsibility for himself' approach, and it hasn't worked.

With that in mind, it seems to me that your rogue programmer is not the problem, he is a victim of the problem. He's the water coming out of a bucket with a hole in it. Well, guess what? water flows, and if you want to keep it in a bucket, you have to make sure the bucket has no holes. Trying to change the water's behaviour is ignoring the hole in the bucket. "Not enough testing due to time constraints" is a problem with your time estimates, or with something based on your time estimates. No testing == broken products, end of story. Anytime that doesn't hold, you can count yourself just as lucky as when you win a few quid from the lottery.

Your rogue's choice, given these restraints and regardless of any disciplinary problems, was to test properly or finish on time, and he chose the latter - he could just as easily have chosen the former, and then you'd have still had nothing to show your client. Ditto with your own testing before your visit. (Remember - discard the grey. If he was off sick for a week during the rush, that is what most people would call a grey area. It's not. It's somewhere to hide from the black and white issue that you don't have a plan to cover something unforeseen like a staff sickness.)

The real problem here is why there wasn't enough time to test, which, without specific detail I can only approach by stabbing blindly at common reasons:

Did you bind yourself to an optimistic time estimate due to a client deadline? Then offer clients x functionality on x date followed by y functionality afterwards. Stand firm on this offer. By all means tell them that you think y will also be ready by x, but don't guarantee it. Walk away if needs be, or there's no point trying to solve these problems in the first place.

Did you bind yourself to a quote based on an optimistic time estimate? Then evaluate your estimation procedure and safeguards, or switch to per-time billing.

Was your contract not tight enough to protect you from delays beyond your control? Go see a lawyer about that spangly new contract you'll be using from now on, the one that doesn't shoot you in the foot every time practice doesn't hold up to theory.

Did you bump the job down to the last minute to focus on other jobs? Take on less and accept the business-success level that that dictates, or invest in a setup that can make more efficient use of your time. (Or raise your prices - some clients will pay, some won't. You have to find the right balance, but it's nearly always possible to earn the same profit by charging more to fewer clients.)

Was it dictated by an un-informed position of authority? That's a whole other ball game, but if that's what happened, you're transferring your failure (to convince the higher-ups of the truth of the situation) onto your rogue programmer. That makes you the bad guy, and makes me on his side :)

Answer 32

I agree with the comments about integration testing, but I would also suggest having a chat with him, as he probably does not realise that he is not very thorough. I have a similar problem - I am very good with thinking in the large (for example, overall architecture), but not very detailed focused in the small. So, for example, if you want to architect a large, complex business transaction, I'm better than most. But if you want a form to collect a user's details and mail them to someone, most developers are more thorough than me. I'm likely to forget null checks, etc.

However, I've learned to compensate for this because I'm aware of this tendency, and so I ensure that I very thoroughly test my code, both with unit tests and additional checks before I think of it as 'done'. But, I had to learn to do that by being made aware of my tendency to brush over the details.

But, be gentle and take his feelings into consideration. Just let him know that you're trying to improve his skills and the team's overall effectiveness.