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.
Best Answer
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.