Software Craftsmanship – Code Reviews

Some basic notes from the discussion on code review practices:

 

Alistair mentioned that his old compny would have a review before each checkin – some were 100s of files. Any team member could approve them, and it ensured consistency.

 

An aside about Pair Programming, several people felt that it was actually quite easy for one partner to just go along with what the other one was doing. This is the ‘driver-navigator’ behaviour. It’s important to be fairly defensive about how well the code works, and the ‘ping pong’ approach with TDD may be better.

 

Gitorious was brought up as a hosting and monitoring solution. Citrix and Perforce allow you to set up a pre-commit hook for review. And there was one example of a code review setup that used a web page to present the code for review. I’m not sure how strong the interface was in terms of diff tools, Intellisense etc.

Other options: Reviewboard, codereviewer, codecollaborator, Atlassian Fisheye.

 

The ‘boyscout principle’ – leave each piece of code a little better than you found it.

 

It’s worth not being too picky in a code review, you shouldn’t be looking at indentation, coding standards etc, if someone hasn’t followed these then that code shouldn’t even enter into code review.

 

And everyone should be responsible for asking for review when appropriate, you shouldn’t have to force people to use a code review process.

 

What do you do with a junior developer that doesn’t know they need review? Some people don’t ask. Especially guys, who are macho and don’t want to show weakness!

One person told us how he made it a policy twice a day to discuss issues with a junior, often in the coffee room in an informal way. This was effective for checking the direction of this developers work.

 

With implementation – if you’ve done it wrong a code review is too late. We heard about a code review process for junior programmers where the code would be put on a projector in a meeting room. If they had gone about it the wrong way it just wasn’t any help.

 

Alistair felt that pairing was vital for the first 3 to 6 months of a developers work on a new company, as there would typically be so much to learn.

 

Facebook – we heard an interesting story about how Facebook developers work, they can push to the live site at any time and are totally responsible for their code quality.  A karma system keeps track of how well this code performs, I’ve found a link detailing this: http://agilewarrior.wordpress.com/2011/05/28/how-facebook-pushes-new-code-live/

 

 

 

 

 

 

Advertisements
Post a comment or leave a trackback: Trackback URL.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Google+ photo

You are commenting using your Google+ account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

w

Connecting to %s

%d bloggers like this: