Making your code easier to review

Code reviews are great. Personally I believe there are many good reasons to do code reviews regularly. Fortunately where I work they are actively encouraged. Unfortunately we don't have a nice tool, like Crucible, to make things a bit easier. SVN log viewer is as good as it gets. This makes code reviews a bit more difficult, so I was wondering, how can I make it easier for others to review my code?

1 - Keep commits small

Seems pretty obvious this one, but it makes a big difference. In a perfect world (i.e. this doesn't always happen) I like to break some my work into a series of steps and commit each step. The smaller the commit the easier it is for the reviewer to see what you were trying to do. Or to put it another way 50 lines of code is much easier to understand than 500.

Ideally a commit can be summarised in a short sentence. If there's paragraphs in the commit message there's probably too much in there. But I have to hold my hands up and say I don't manage it all the time.

2 - What and why

Something else I find helpful in commit messages is the why. What has been done is nearly always covered but the why is often missed. Few things are more frustrating than reading a commit message that says something like "removed xyz from system". That's nice, I can see what you've done but why did you do it? I can't see the reason! Tell me! Why tells you if the work actually needed to be done. Why explains the reason behind an obscure code change. Why tells you the motivation behind the work. Why is important when you're reviewing code.

3 - Avoid noise

Code changes are easier to review if they only contain the important stuff. For example, changes to the logic of a method or the changes to a LINQ query or a bug fix. Unfortunately commits often contain lots of noise. By noise I mean anything that isn't directly related to the piece of work being undertaken. This includes things like lots of white space changes, code alignment, variable renaming, method renaming or any other refactoring. This stuff should definitely still be done, but ideally in a separate commit so the reviewer doesn't have to work out what is noise and what is not.

4 - Check your changes

I'm always surprised, not sure why, by the number of daft mistakes I pick up when I check my code changes before each commit. By this I mean view the diff for each individual file I'm committing. Going back to my first point, this is much easier if the commit is small. Typically I will find unnecessary comments, debugging print statements, test code, files I forgot to revert, code I temporarily commented out etc. etc. If you spot these problems the reviewer doesn't have to.