我们还需要争执是否需要代码审查么？

转自DNN团队成员博客：

http://www.dotnetnuke.com/Resources/Blogs/EntryId/3369/Peer-Code-Review-ndash-Are-they-worth-the-hassel.aspx

原文如下，希望对大有所启示

PeerCode Review – Are they worth the hassel?

by Ash Prasad on Friday, May 04, 2012 8:30 PM




This blog has been cross-posted from my personalblog

Over the lastmany months the Engineering Organization at DotNetNuke Corp. has been makingmany process changes to deliver high-quality Software. As Scrum Master and LeadDeveloper, I’d like to dedicate a blog series on what we did, how it helped andthe lessons
we learned.

In this blog,I’d like to talk specifically about Peer Code Review that we implemented over10 months ago.

Why need PeerCode Review:

Before I go anyfurther, I must admit that the DotNetNuke Corp. has one of the finestEngineering team I’ve ever worked with; team is not only technically sound (feware actually Microsoft MVPs), but also has great work ethics, passion for whatthey do and above
all great team work.

Might you askif you’ve got such great people, why need Peer Code Review. Good question, hereis why:

As you all knowDotNetNuke Framework has been around for many years and is regularly underworks for delivering yet exceptional product. At times Engineers end upchecking-in code that results in unintentional breakage elsewhere. Some of thecommon breakage
are build, package, unit test, automated test and softwarefunctionality. Simply put, despite their best efforts, it’s just not possiblefor everyone to know everything about the framework.

There must be abetter approach to move aggressively but also keep higher code quality.

What’s thesolution – Implement Peer Code Review:

As a team weanalyzed the problem and came up with Peer Code Review as an inexpensive way toimprove our code quality. After all the wise men have once said, one and onecombined is eleven


Why not usethe power of two. We made a rule that every check-in must be code-reviewed byanother Engineer.

Note theemphasis on word “Peer”, it’s not Manager, Lead or anything else, just a peer,which is another Engineer in the team.

In theDotNetNuke engineering organization, we have a concept of buddy-system, whichis actually very helpful. More on this on another blog.

How does itwork (logistics)

When anEngineer makes code change and checks-in code in TFS, he/she sets the CodeReviewer field TBD followed by Engineer name and notifies to that person byemail.




ReviewingEngineer analyzes the code and puts their name in TFS changeset (replacing theword TBD) to indicate code review is done. If a problem is found the twoEngineers work together to resolve and check-in more code as needed. Thesubsequent checks-in also
needs to be reviewed. It’s an iterative process.Given, we’ve been doing this for many months there is usually less rework.




Initial TeamReaction

Most of theEngineers liked it on day-one as they can sleep well in night knowing theircode is being reviewed by another peer. A few were hesitant as now everycheck-in must get approved and was a bit hard to digest. However, after a fewweeks everyone bought-into
it and made it a habit. I personally feel insecureif there is no code-review on my own code, call it self-code-phobia


Enforcement

We didn’treally have to do a whole lot to enforce this new rule; every few weeks we’drun a report (custom program) in TFS to see which changesets didn’t have areviewer. Their names were called-out and asked to complete the code-review.This was done just
a handful of times and after that it got into auto-pilotmode. However, as Scrum Master and Lead Developer I keep reminding people tokeep doing code-reviews as it’s very easy to not do as well.

Not everythingneeds Code Review

Trivial changessuch as adding comments, fixing broken tests or typo-correction doesn’t requirecode review. It’s optional, some do some don’t.

Is it a burdenon time?

Definitely not.We don’t spend a whole lot time doing code review, but at times it can takeover 30 minutes or an hour (or even two) a day, especially if the reviewer hasbeen procrastinating and has a pile of reviews to be done


Many timesyou’d here on daily standups as one of the main activities in last 24 hours. Ifyou are on top of this, it’s 10-15 minutes a day.

When does itwork

This is themost important part of this blog. It works when the person doing code-review iswell versed with the code being asked to review. For e.g. two Engineer workingon same area. On top of that if the other Engineer is waiting for your changesto do his
stuff is also super efficient. That’s where I feel it works the most,when the other person not only does code review but also executes your codewhile testing their own stuff. We insist Engineers to work in teams of two;definitely on complex projects; however,
it’s not always possible.

When it doesn’twork

Something iswrong when you are hunting for a reviewer:) Ideally there should be a naturalchoice to select the reviewer. We’ve run into cases where the person makingcode changes is the only person working in that area. In such situationsreview-task is assigned
to anyone on the team; the reviewer is expected to dotheir best and call out issues. In such cases you need to insist QualityEngineers to be extra diligent in their QA process.

Anothersituation where code review is not that effective is when Reviewer is too busy withother work. Reviewer might rush through the review doing superficial scrutinyonly. One needs to be careful while assigning Reviewer.

Review qualitycan sometimes get impacted when Reviewer and Reviewee are in differenttimezones. We haven’t had many problems due to this reason, however, it’spossible.

Is Code Reviewsufficient for Quality Code

Absolutely Not.This is a mandatory step that every team should incorporate in their Softwaredelivery process. Great Software requires lots of other things (process,people, tool, etc.). More on other blogs.

Summary

Peer codereview is essential to minimize bugs and maintain high-quality software. Itworks best when Reviewer and Reviewee are working on the same area or are wellversed with the code.

Please feel freeto share your thoughts as comments in this blog.