13

First allow me to coin a term:

code goal-tending: Checking out code in the morning, then silently reviewing all of the changes made by the other developers the previous day file by file, (especially code files you originally developed), and fixing formatting, logic, renaming variables, refactoring long methods, etc., and then committing the changes to the VCS.

This practice tends to have a few pros and cons that I've identified:

  • Pro: Code quality/readability/consistency is often maintained
  • Pro: Some bugs are fixed due to the other developer not being too familiar with the original code.
  • Con: Is often a waste of time of the goal-tending developer.
  • Con: Occasionally introduces bugs which causes hair-pulling rage by developers who thought they wrote bug-free code the prior day.
  • Con: Other developers get aggravated with excessive nitpicking and begin to dislike contributing to the goal-tender's code.

Disclaimer: To be fair, I'm not actually a development manager, I'm the developer who is actually doing the "goal tending".

In my defense, I think I'm doing this for good reason (to keep our extremely large code base a well oiled machine), but I'm very concerned that it's also creating a negative atmosphere. I am also definitely concerned that my manager will need to address the issue.

So, if you were the manager, how would you address this problem?

UPDATE: I don't mean for this to be too localized, but some have asked, so perhaps some background will be illuminating. I was assigned a giant project (200K LoC) three years ago, and only recently (1 yr ago) were additional developers added to the project, some of which are unfamiliar with the architecture, others who are still learning the language (C#). I generally do have to answer for the overall stability of the product, and I'm particularly nervous when changes are surprisingly made to the core architectural parts of the code base. This habit came about because at first I was optimistic about other developer's contributions, but they made way too many mistakes that caused serious problems that would not be discovered until weeks later, where the finger would be pointed at me for writing unstable code. Often these "surprises" are committed by an eager manager or a co-worker who is still in the learning phase. And, this probably will probably lead to the answer: we have no code review policy in place, at all.

Improve this question
10
  • 3
    Apparently your developers aren't pumping your tires enough. Replace your goaltender with FxCop or something similar and enforce those standards automatically so they can't check in.
    – Jon Raynor
    Mar 16, 2012 at 15:06
  • 2
    Con: It's not your job. These folks should be doing their own goal-tending.
    – Robert Harvey
    Mar 16, 2012 at 15:12
  • 10
    as a developer I would find this infuriating for another developer to do this with all the changes I make, and if bugs are reintroduced/introduced as a result then its completely unacceptable
    – Ryathal
    Mar 16, 2012 at 15:13
  • 4
    @Ryathal: The shop should be enforcing a code standard. Code is generally owned by the whole team, so if you don't want people changing it, you should make it right in the first place.
    – Robert Harvey
    Mar 16, 2012 at 15:14
  • 1
    @RobertHarvey enfocing a standard is one thing, a rogue developer silently enforcing his view of "right" is a different matter, and this seems to be a case of the latter.
    – Ryathal
    Mar 16, 2012 at 15:22

4 Answers 4

Reset to default
53

It sounds like what you're doing is basically equivalent to a code review except that rather than providing feedback to the developer, you're making all the changes that you would suggest in a code review. You'd almost certainly be better off doing an actual code review where you (or someone else) provides feedback to the original developer about code quality issues and obvious bugs and ask the original developer to fix them. That keeps code quality up but it also helps the developer become more familiar with the original code and its pitfalls and helps improve future code changes. Plus, it doesn't have the downside of causing "hair-pulling rage" when a bug gets silently introduced or cause other developers to think that they're being talked about behind their back.

Improve this answer
6
  • 4
    Big +1. Code reviews help build the team while the "goal tending" he describes seems like it might rub people the wrong way.
    – Doug T.
    Mar 16, 2012 at 16:34
  • 2
    This. Real code reviews would be a much better path here. The two big upsides being, like you said, the developer learns the application architecture more quickly because you are showing him the issues. The second being that you won't be introducing bugs because you don't fully understand the new code being written. Perfect answer for this question.
    – Mike Cellini
    Mar 16, 2012 at 16:37
  • 2
    Brilliant answer and thanks for the insight. I think a bringing copy of this question and a humble attitude to my manager might convince him that code reviews would be beneficial to the team and will reduce the need for "goal tending".
    – Kevin McCormick
    Mar 16, 2012 at 17:05
  • 1
    Agreed. Don't muck with other people's code without asking. You can get some gnarly bugs that way. I've fixed goal-tending bugs, and they are not pleasant. If you are concerned about code robustness, write unit tests.
    – Paul Nathan
    Mar 16, 2012 at 20:49
  • 2
    Even if you never transition to "real" code reviews, simply talking to the other person - asking why they did certain things, and why they didn't do it [some other way], is a great way to accomplish many of the same goals. +1
    – TehShrike
    Mar 17, 2012 at 0:13
14

To be frank, IMHO, this is a terrible idea.

I would expect morale to drop into the gutter, if it hasn't already.

To be fair to you, you do acknowledge this.

Peer code reviews are fine, it puts the onus on the devs all at one level, instead of it being a "them versus us" scenario. (where them is management/leads).

There may be some developers that you may need to keep an eye on more than others, but blanket forcing all code to come through you first is ridiculous.

And despite how good you think you are, there will be times when you are wrong, or "nit picking" and this will just make things even worse.

Improve this answer
1
  • That and the manager is probably more likely to make the code worse.
    – Andy
    Mar 17, 2012 at 1:52
5

If I were the manager, to address this problem:

  • Review code standards and practices with team, hand them a copy of the coding standards
  • Peer review code on a regular basis to reinforce the standards and practices that are to be followed
  • Enforce nitpicking/formatting with an automation tool so that developers are forced to follow the standards
  • Get rid of any bad teammates who refuse to follow standards
  • Stop being the goaltender

Your intentions are good, but the implementation is terrible and as others have pointed out, will cause poor morale and sniping among team members.

If the project never had any standard to begin with, try to ease in the new standard in stages instead of just flipping a switch.

Improve this answer
3

Bad juju.

I'm not a development manager, but if I were I would not want any of my developers to touch code that isn't part of a defect or feature assigned to them. Period. If you see a problem with someone else's code, then by all means bring it to the attention of the developer(s) responsible, but do not just dive in and fix it yourself, especially if you haven't coordinated with the other developer(s) first.

Cleanup tasks should only be executed after a formal code review by the team, and only by the developer(s) to whom those tasks have been assigned.

Initiative is good in general, but sometimes it can bite you in the ass. If you introduce any bugs into what was working code, you may rapidly be wishing you'd chosen a different career path.

Improve this answer

Your Answer

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

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