Ayende @ Rahien

It's a girl

Code Reviews are not just about the code

Usually when I am doing a code review, I ask not just access to the code, but to the source control repository as well. There are quite a lot that you can learn from the source control of a project.

Here is something that is both good and bad. It is bad because we have no checkin comments. It is good because we can see that we have very frequent checkins.

image 

Another thing that you want to check is the bug tracking system. Is there such a thing (including email thread or a post it notes or a story wall)? Is it relevant?

Is there a defined build process? How do you deploy to production? How do you manage DB changes? What is you back off strategy from an ambitious feature? How do you rollback a failed deployment? Do you have a physical deployment plan?

I don't expect all projects to have detailed answers to the all the questions I put above, but I do expect to have a response. The worst possible thing that could happen is that there is no thought given to those.

My standard answer to "How do you manage DB changes?" is: I create a diff using SQL Compare and apply that when needing to update DB versions.

The most important thing in my opinion is the actual interaction in code reviews. I tend to have two stages of code review. The first is going over the code with only general idea about what is going on. This is to find if there are things that require special attention, if there are obvious issues there.

In general, you want to be able to view this from a clean mind. Then, you want to have a conversation with the developers. Based on that initial review. This allows you to capture more than just the code, it let you capture the actual dynamic of the team and how they work. During this phase I also get a lot more

The second phase of the code is much more in depth, I tend to try to go over every line in the code at least once. After that, I can sit with the developers again and discuss the overall impression.

Probably the most value in the process comes at this second state, I think.

Comments

Thomas Krause
07/31/2008 10:09 PM by
Thomas Krause

I found multiple products by the name of "SQL Compare". Can you give me a link or company name? I've desperately searched for a good Database Diff program some time ago, but did not find a real good one. I've even written my own little tool to help me with these tasks....

Thanks!

Ayende Rahien
07/31/2008 10:11 PM by
Ayende Rahien

Thomas,

I tend to use Red Gate's tools.

Rik Hemsley
08/01/2008 08:32 AM by
Rik Hemsley

My svn post commit hook sends an email to all developers (there aren't many of us) with the files changed and the description. If the developer didn't write a description of the change, it says 'No description given. Naughty developer!'

Shaming works quite well. They generally write descriptions now.

Vishwajeet Singh
08/01/2008 11:41 AM by
Vishwajeet Singh

even a check in precommit can be added to check log message, but there is no point in enforcing things, developer should be aware of side effects of not adding log messages; that awareness is critical

andhapp
08/02/2008 09:10 PM by
andhapp

Apex SQL diff is a good tool for comparing SQL databases.

Pr0fess0rX
08/10/2008 08:43 AM by
Pr0fess0rX

Do not forgot that Microsoft Visual Studio 2008 Database Edition have the ability to Compare either Schema or Data and it's similar to Red Gate tools, I advice to try it also.

Comments have been closed on this topic.