out that reviews may be superfluous
if the project practices pair programming. Others note that when it comes
to finding code flaws (such as a lurking
buffer overflow) static analysis tools
are more effective than human inspection. In any case, the process is time-consuming; most teams apply it on the
entire code, as well as on samples. Still,
code reviews remain an important tool
in the battery of accepted “best practices” for improving software quality.
Our group has found that the exercise is indeed useful when adapted to
the modern world of software development. The first extension is to include
design and specification. Many recent
references concerning code reviews focus on detecting low-level code flaws,
especially security risks. This is important but is increasingly a task for
automated tools, not for humans. Our
experience suggests that abstract program interface (API) design, architecture choices, and other specification
and design issues are just as worthy of
a reviewer’s time and play an increasingly important part in our reviews.
Among these traditional principles, one that should definitely be retained in the new distributed context
is that reviews must focus on identifying deficiencies, not attempt to correct them. With the advent of better
software technology it may be tempting to couple review activities with
actual changes in software repositories; one environment that supports
Web-based review—Code Collaborator, www.smartbear.com—allows this
by coupling the review tools with a
configuration-management system.
Such coupling may be risky; updating
software—even for simple code changes with no effect on specification and
design—is a delicate matter best performed in an environment free from
the time pressure inherent in a review
session.
figure 2: excerpts from a chat session during a review (names blocked out).
ISE weekly meeting
says:
Good morning/evening
21-Feb-08 17:00: 45
says:
21-Feb-08 17:00: 46
Hello
says:
For info: the doc’s url as preview: http://docs.google.com/
View?revision=_latest&docid=dd7kn5vj_8gmxzhffv&hl=en
21-Feb-08 17:04: 21
says:
21-Feb-08 17:05: 28
there is an echo
never mind
21-Feb-08 17:05: 48
says:
I disagree.
When there is a crash, if the we have multi lines, then we can know exact
error point. If we write them in one line, then we have to guess.
21-Feb-08 17:17: 50
says:
we need to improve the RTNHOOK macro
if we improve it that it won’t be a problem
RTHOOK ( 1)
21-Feb-08 17:18: 45
21-Feb-08 17:18: 55
21-Feb-08 17:19:00
says:
we have bp slot index; . .we would need to show the “nested bp slot index”
that’s possible ... somehow
21-Feb-08 17:19: 10
21-Feb-08 17:19: 17
says:
RTNHOOK ( 1, 1); /* First instruction, first nested or expression */
21-Feb-08 17:19: 25
says:
Ok if we have the ‘nested bp slot index’ issue.
Ok if we have the ‘nested bp slot index’ feature.
21-Feb-08 17:20: 15
21-Feb-08 17:20: 26
says:
It’s possible to view expressions in the debugger.
21-Feb-08 17:21: 48
says:
indeed sometime doing the evaluation is not desired due to potential side effects)
21-Feb-08 17:27: 14
says:
I was just curious of clear rules about IEK. 5. 1
21-Feb-08 17:28: 19
Distributed Review
All the descriptions of code reviews
I have read in the literature present a
review as a physical meeting among
people in the same room. This is
hardly applicable to today’s increasingly dominant model of software development: distributed teams spread
over many locations and time zones. 3
At Eiffel Software, we were curious to
see whether we could indeed apply the
model; our first experience—still only
a few months old and subject to refinement—suggests that thanks to the Internet and modern communications
technology a distributed setup is less
a hindrance than a benefit and that
today’s technology provides a strong
incentive to revive and expand the idea
of the review.
Though the EiffelStudio development team includes members in
California, China, Russia, and Western Europe, it still manages to have a
weekly technical meeting, with some
members agreeing to stay up late; for
example, in the winter, 8 a.m. to 9 a.m.
in California means 5 p.m. to 6 p.m.
in Western Europe, 7 p.m. to 8 p.m. in
Moscow, and midnight to 1 a.m. in
Shanghai (see Figure 1). We devote every second or third such meeting to a
design and code review.
Although many of the lessons we
have learned should be valid for any
team, some of the specifics of our reviews may influence our practice and
conclusions. Our meetings, both ordinary ones and those devoted to reviews,
last exactly one hour. We are strict on
the time limit, obviously because it’s
late at night for some team members
but also to avoid wasting the time of
a group of highly competent develop-