|
Somewhere along the way, I picked up this habit of where I go through each file I'm about to check-in and diff it with the current repository version. In theory, this isn't really necessary - since I've synced to the latest version and tested the build, the files should be good to commit. Practically speaking though, reviewing the diffs is good for a number of reasons.
First, it makes me mentally run through all of the work I did to get to this point. At very least, this helps me write a useful check-in comment. Very often, it also reminds me of something I meant to go back and finish or fix before committing.  This avoids embarrassing check-ins, so the rest of the team nevers sees those horrifying hacks I used "just the get things going".
Perhaps more importantly though, looking over the changes I'm about to apply to the repository makes me think about how applicable these changes are to the rest of the team. Many times, I've caught config options or hard-coded "test" values that only make sense for me on my computer, and would break the game for someone else. This kind of stuff doesn't show up when running tests locally. It's sort of like proactive bug fixing - instead of encountering a bug and trying to track down the source, I'm reviewing each change thinking, "how could this break something?"
I'd like to say that I developed this habit through a dedicated effort to improve software quality. In reality, I think it started because (a) I'm really forgetful when it comes to the code I've written, so can't remember what I just did that I'm trying to check-in, and (b) TortoiseSVN's commit dialogue makes it super easy to do. You just double click on a file in the change list, and it opens it up in your visual diff program of choice. Perforce does something similar (in P4V anyway) when you hit Ctrl-D.
It feels pedantic and even borderline neurotic every time I do it, but reviewing my diffs has definitely contributed to the quality of my code. It's worth a try, maybe as one of the easiest New Year's resolutions ever!
|
Since my first day on the job, I've gone through every changelist, checking every difference in every file, before every checkin. Maybe I'm really neurotic and pedantic too, but I couldn't do it any other way. I consider it absolutely mandatory.
Although I suppose this was ingrained by the peer-reviewed checkins we did at Outerlight - where we went through every file in every changelist showing another coder what we'd done and briefly explaining why. This may seem like overkill to some people - it did to us, on occasion - but the way it forces you to think about what you're showing the other person and think back to why you did things the way you did is highly valuable. Even if the other person doesn't directly contribute or spot mistakes, it makes it very likely that YOU will. It also alters your mindset somewhat when you know you're going to have to show someone your 'secret shame' - I really think it drove up the quality of our submissions.