Kishore
SCJP, blog
What might your book have to say about such a process for code reviews?
Small, frequent code reviews keep your code clean, simple, and tidy.
You can avoid the traditionally unpleasant code reviews that involve
dozens of developers and require days of preparation (a.k.a. The Mighty
Awful and Dreaded Code Review, hereafter referred to as MAD reviews
for your reading enjoyment). We�ve found code reviews can be painless
when you adhere to the following rules:Only review a small amount of code. There are one or two reviewers at most. Review very frequently, often several times a day.
Check out <b>Ship It! A Practical Guide to Shipping Software</b><br /> <br /><a href="http://www.pragmaticprogrammer.com/titles/prj/" target="_blank" rel="nofollow">http://www.pragmaticprogrammer.com/titles/prj/</a>
Originally posted by Jason Menard:
What might your book have to say about such a process for code reviews?
Check out Ship It! A Practical Guide to Shipping Software<br /> <br /><a href="http://www.pragmaticprogrammer.com/titles/prj/" target="_blank" rel="nofollow">http://www.pragmaticprogrammer.com/titles/prj/</a>
Originally posted by Jason Menard:
Some good feedback, thanks. I certainly agree that the small frequent reviews are the way to get the most benefit. Is there any method you use to figure these code reviews into the schedule?
Check out <b>Ship It! A Practical Guide to Shipping Software</b><br /> <br /><a href="http://www.pragmaticprogrammer.com/titles/prj/" target="_blank" rel="nofollow">http://www.pragmaticprogrammer.com/titles/prj/</a>
Originally posted by Jared Richardson:
When things are going well a code review will take 5 to 15 minutes, not enough time to bother scheduling. When we say small and frequent, we mean it!
[OCP 17 book] | [OCP 11 book] | [OCA 8 book] [OCP 8 book] [Practice tests book] [Blog] [JavaRanch FAQ] [How To Ask Questions] [Book Promos]
Other Certs: SCEA Part 1, Part 2 & 3, Core Spring 3, TOGAF part 1 and part 2
Originally posted by Jeanne Boyarsky:
Does that include pseudo code reviews that people do as they pull in code from the repository or look at it for other reasons? These tend to be below the level of what is called a code review, yet there is certainly code reviewing going on.
Check out <b>Ship It! A Practical Guide to Shipping Software</b><br /> <br /><a href="http://www.pragmaticprogrammer.com/titles/prj/" target="_blank" rel="nofollow">http://www.pragmaticprogrammer.com/titles/prj/</a>
MCP, MCP+I, MCSE(NT4), MCSE+I, MCSE(2000), MCDBA, MCSD(VS6)<br />SCJP 5.0, SCBCD 1.3<br />ICED(v5.0), ICSD (WSP5.0)
Originally posted by Tony William:
Just to share my experience in doing code review.
As the team lead, I alone is the one doing small frequent code reviews at the beginning. Then later on, I have another more experience staff helping me..... Now (after 1 year), most of the members in the team get used to it in reviewing code from others. Even not being told, they will voice out to each other if they spot something wrong.... Well, in most cases, I would say the code review is done for QA purposes (I am not sure if this is code review anymore)
I find this a good way to share the knowledge & build up the skills in the team.
When you introduce the code review process, you may need to appoint
a few senior team members to be the mandatory reviewers; one of
the senior team members must participate in every review at first.
You shouldn�t need them to continue in this role for more than a few
months. Once your team members learn the basics, the whole team
will be capable of sharing the responsibility. As the proverb says, �As
iron sharpens iron, so one man sharpens another.�9 The point is for
the team members to work together and so improve each other. Involve
your team members in the sharpening process as quickly as possible.
We worked in one shop that really illustrates how code reviews can
be used to leverage your senior members. We had three very senior
developers and five who were decidedly not�they weren�t rank novices,
but sometimes they had peculiar ideas of how to fix a problem. In order
to protect the product and to bring the junior developers up to the next
level, all code reviews involved one of the senior team members. This let
the more experienced team members instruct and teach while catching
problems before they were introduced into the product. It also made
the senior team members aware of misunderstandings and real issues
that the junior developers faced.
These reviews were a great help to the team. We frequently spotted
repeated code and summarily pulled it out and moved it into utility
classes. Reviewers caught and removed code that had nothing to
do with assigned work (otherwise known as freelance refactoring) and
rejected uncommented code outright. As the team moved forward, an
imperceptible (but very important) change took place.
Each of the junior team members started picking up good habits, one
code review at a time. They started cleaning up code before the reviews,
adding meaningful variable names, comments, and such before they
were asked. Long, cumbersome routines became short and manageable.
Even better, the lessons taught in the code reviews stuck. After about
three months, we changed the code review policy so that any team
member could do the reviews.
Check out <b>Ship It! A Practical Guide to Shipping Software</b><br /> <br /><a href="http://www.pragmaticprogrammer.com/titles/prj/" target="_blank" rel="nofollow">http://www.pragmaticprogrammer.com/titles/prj/</a>
MCP, MCP+I, MCSE(NT4), MCSE+I, MCSE(2000), MCDBA, MCSD(VS6)<br />SCJP 5.0, SCBCD 1.3<br />ICED(v5.0), ICSD (WSP5.0)
And once the review is over,
show the rest of your team what you�ve done with code change notifications.
SCJP 1.4, SCWCD 1.4, SCBCD 1.3, SCJA<br /> <br />blog: <a href="http://jroller.com/page/rharianto" target="_blank" rel="nofollow">http://jroller.com/page/rharianto</a>
Originally posted by Rudy Harianto:
Jared,
This was taken from the intro from your book:
I wanna know what kind of notifications does it means? through email or oral? How important is this notifications?
Cause from your suggestion that we should review just a small amount of code, so wouldn't it be a lot of notifications that should be made?
When you edit code, an automatic build system can notice the change
and rebuild the project (see Practice 4, Build Automatically,, on page 30).
Your next step is to publish that information so that every member of
the team knows what changed.
A change notification system pushes this information out to your entire
shop, not just your immediate co-workers. The effect from this type of
knowledge sharing can be quite amazing.
Similar to Alistair Cockburn�s �information radiators,� you are making
information available. Your team can use it or ignore it, but the information
is being put out there for you. In fact, this practice doesn�t
make you fetch the data; it pushes it to your desktop.
Each time we�ve introduced this practice, a sizable percentage of the
shop has been opposed to the practice before they�ve tried it.
After about a month, the worst complainers always come by to apologize
and tell us how useful the tool has become to them. This technique
consistently brings the most resistance, but after a short time everyone
becomes acclimated to having the notifications available. It quickly
becomes a vital resource.
Check out <b>Ship It! A Practical Guide to Shipping Software</b><br /> <br /><a href="http://www.pragmaticprogrammer.com/titles/prj/" target="_blank" rel="nofollow">http://www.pragmaticprogrammer.com/titles/prj/</a>
Originally posted by Jared Richardson:
The code reviews in the book specifically talks about the interaction between two (or more) developers. A big benefit of the review process is the knowledge sharing that goes on. One developer showing others what he did, explaining his intentions, sharing stylistic conventions, etc.
There's certainly nothing wrong with learing from code from the repository (or Google!), but I'm not sure I'd call it a code review.
[OCP 17 book] | [OCP 11 book] | [OCA 8 book] [OCP 8 book] [Practice tests book] [Blog] [JavaRanch FAQ] [How To Ask Questions] [Book Promos]
Other Certs: SCEA Part 1, Part 2 & 3, Core Spring 3, TOGAF part 1 and part 2
MCP, MCP+I, MCSE(NT4), MCSE+I, MCSE(2000), MCDBA, MCSD(VS6)<br />SCJP 5.0, SCBCD 1.3<br />ICED(v5.0), ICSD (WSP5.0)
Did you see how Paul cut 87% off of his electric heat bill with 82 watts of micro heaters? |