1. No category

Definitions
Modern'Code'Review
Modern Code Review
• Wikipedia:*
“Code*review*is*systematic*examination*(often*known*as*peer*review)*of*
computer*source*code.*It*is*intended*to*find*and*fix*mistakes overlooked*
in*the*initial*development*phase,*improving*both*the*overall*quality*of*
software*and*the*developers'*skills.”
Expectations, Outcomes & Challenges
Cohen, Jason. “Chapter 18: Modern Code Review” in Greg Wilson and Andy Oram.
Making Software: What Really Works, and Why We Believe It. New York. 2010.
• Bacchelli &*Bird:*
• Modern code*review*is*characterized*as*informal*and*lightweight,*uses*
tools*to*facilitate*the*process,*and*occurs*regularly*in*practice.*
Bacchelli, Alberto and Christian Bird. “Expectations, outcomes, and challenges of modern code review”.
Proceedings of the 2013 International Conference on Software Engineering (ICSE '13). New York, 2013.
202-211.
Jeff Avery
CS 846: Topics in Empirical Software Evolution
Mar 19, 2015
3
Introduction
Making'Software
Motivation
Ch. 18: Modern Code Review
Jason Cohen
• Question:*“Every*page*in*this*book*has*been*checked*over*by*an*
editor.*Why?”
• Answer:*“It’s*impossible*to*write*prose*without*independent*scrutiny,*
surely*it’s*also*impossible*to*write*code*in*isolation;*code*has*to*be*
correct*to*the*minutest*detail,*plus*it*includes*prose*for*humans*as*
well!“
Blogger,*Entrepreneur,* Author
Lots*of*articles* on*“Effective*Code*Review”
http://blog.asmartbear.com/jasonQcohenQresume
• Cohen:*Writing+code+is+a+lot+like+writing+a+book.+Code+reviews+are+the+
way+that+we+ensure+correctness!
4
5
Introduction
Introduction
What’s'the'Cost?
Research'Questions
• Code*Reviews*are*a*timeQintensive*process.
• CodeQauthor:*time*to*prepare*code*and*supporting*documentation,*
send*out*notes*ahead*of*time,*organize*the*meeting.
• Reviewers:*Time*to*prepare,*sit*in*a*review*for*1Q2*hours,*provide*
feedback.
• Is*there*enough*of*a*benefit*to*justify*the*cost?
• “*Executed*properly,*code*reviews*find*bugs*faster*than*testing*or*other*
known*debugging*techniques—but*when*done*inefficiently*they*can*
quickly*become*unproductive.”
Two*main*questions:
1. What*factors*lead*to*an*effective code*review?
2. What*are*the*best+practices+around*code*reviews?*
6
Factors
7
Factors
Focus'Fatigue'(Time)
Inspection'Rate'(Speed)
• People*can*only*maintain*intense*
focus*for*soQlong*before*their*
attention*drops.
• Going*tooQfast*prevents*
reviewers*from*spotting*
defects.
• The*rate*of*discovery*drops*
significantly* after*about*60*mins.
• Code*varies*in*complexity*
so*variation*is*expected.
• DropQoff*at*400Q500*LOC*
explained* as*the*reviewer*
“going*too*fast*to*find*
defects”.
• Conclusions
• CodeQreviews* can*be*highQvalue.
• They’re*tiring.
Defect*detection*rate*drops*at*around*400Q500*LOC*inspected/hour.
Average*rate*of*defect*discovery.*After*60*mins,*the*
rate*of*discovery*drops*noticeably.*Dunsmore et*al.*[4].
8
9
Factors
Factors
Size'Kills
Context
• If*we*can*
• (a)*only*review*code*for*1*hour*
effectively,*at*
• (b)*a*density*of*no*more*than*
400Q500*LOC,*then*
• (c)*we*shouldn’t*review*more*
than*400*LOC*per*review.
• Defect*rates*noticeably*dropQoff*
at*400*LOC.
• How*important*is*it*to*perform*code*reviews*with*related*files,*
classes,*documentation,*and*other*context*(compared*to*just*a*diff of*
the*code*changes?)
• Dunsmore 2000:*
• Automated* methods* for*finding*related*files:* 15%*more*defects
• Person*making*a*judgment*call*about*related*files:*30%*more*defects.
• Context*is*important.*
Cohen:*“[When+performing+a+code+review]+you+need+to+be+in+front+of+your+IDE+
with+the+entire+codebase+at+your+fingertips.”*[1]
Detect* detection*rates*drop*as*code*size*increases.*[5]
10
Best*Practices
11
Best*Practices
Are'Meetings'Required?
PeerJReviews
• Fagan*1976:*meetings*are*
essential*to*get*“synergy”.
• Votta 1993:*measured*defects*
found*before and*during the*
code*review*meeting.
• “Are*twoQheads*always*better*
than*one?”
• Cisco*study*comparing*300*selfQ
reviews*vs.*300*peer*reviews*[5].
• SelfQchecked* code*had*half*the*
defect*density*(i.e.*50%*as*
effective).
• The*defects*found*in*meetings*
are*more*“subtle”.
• 25%*of*the*“defects*from*
reading”*were*false*positives!*
Authors* typically* find* half* of*their* own* defects.
Meetings*contributed*4%*of*the*defects*found*[6]**
12
13
Conclusions
Expectations,'outcomes,'and'challenges'of'
modern'code'review
WrapJup
“When*is*code*review*useful,*and*what*are*the*best*practices?”
Proceedings'of'the'2013'ICSE.'New'York,'2013.'202J211.
Observation
Best-Practices
Authors* can*selfQidentify* about* 50%*of* detectable*
defects* (compared* to*peerQreview).
Authors* should review* their*own* code* prior to*preparing*
for*a*formal* code* review.
Most* defects found* by*reviewers* are*discovered* prior+ Before*to*the*meeting:* the*author* should provide* as*much*
to*the*code* review*meeting.
supporting* material* as*possible* (incl.* checkedQinQcode),*
and* reviewers* should* attempt*to*identify* issues.*
…*but* 25%*of* the*issues* discovered* prior* to*the*code*
review* meeting* are*falseQpositives.
Focus the*meeting* around* clarification* of*issues* (instead*
of*a*code* walkthrough).
Defect*identification in*peerQreviewQmeetings* is*only*
effective* for*1*hour* (up* to*a*rate*of*400* LOC/hour).
Only* schedule* short* code* review* meetings.
Alberto*Bacchelli
Assistant* Professor,* TU*Delft
https://scholar.google.ca/citations?
user=ozQbPWoAAAAJ& hl=en
14
Introduction
Christian* Bird
Microsoft* Research
https://scholar.google.ca/citations?u
ser=aDVlSQwAAAAJ& hl=en
15
Introduction
Motivation
Research'Questions
• The*authors*are*interested*in:*
“…practices*in*teams*that*use*modern+ code+review,*revealing*what*
practitioners*think,*do,*and*achieve”.*
1. What*are*the*motivations*and*expectations*for*modern*code*
review?*Do*they*change*from*managers*to*developers*and*testers?*
2. What*are*the*actual*outcomes*of*modern*code*review?*Do*they*
match*the*expectations?*
3. What*are*the*main*challenges*experienced*when*performing*
modern*code*reviews*relative*to*the*expectations*and*outcomes?*
• To*expose*these*practices,*they:
“…*empirically*explore*the*motivations,*challenges,*and*outcomes*of*
toolQbased*code*reviews”*through*a*series*of*observations,*
interviews*and*surveys*with*programmers*and*managers*at*
Microsoft.*
16
17
Methodology
Methodology
CodeFlow'@Microsoft
Mixed'Approach
• Collaborative*code*review*tool*
developed*at*Microsoft.
• 40,000*developers*at*time*of*
publication;*now*in*Visual*Studio.
• Reviewers*can*directly*annotate*code,*
and*interact*with*participants..
• Developers*who*want*a*review:
• Create*a*package*with*the*changed*
(new,*deleted,*and*modified)* files.
• Select*the*reviewers.*
• Write*a*message*&*description.
• Submit*everything*to*the*CodeFlow*
service,*which*notifies*via*email.*
Mixed* approach:* (1)*analysis* of* previous* study,* (2)*observations* and* interviews*
with* developers,* (3)* card*sort*on* interview* data,*(4)* card*sort*on* code* review*
comments,* (5)* the*creation*of* an*affinity* diagram,* and* (6)* survey* to*managers* and*
programmers* [2].
CodeFlow screenshot* [2].
18
Results:*Why*Do*Programmers*Do*Code*Reviews?
19
Results:*Why*Do*Programmers*Do*Code*Reviews?
Question'1:'Motivation
A.'Finding'Defects
• “Our*first*research*question*seeks*to*understand*what*motivations*
and*expectations*drive*code*reviews.”
• The*authors*examine*the*results*of*the*observations*and*interviews.
• “The*reason*why*code*
inspections*were*devised*in*
the*first*place:*reducing*
software*defects,*and*
addressing*correctness”.
• Developers*see* code*reviews*as*an*activity*that*has*multiple beneficial*
influences…* for*the*team*and*the*entire*development* process.*
• “[code*review]:+(1)+makes+people+less+protective+about+their+code,+(2)+gives+
another+person+insight+into+the+code,+so+there+is+(3)+better+sharing+of+
information+across+the+team,+(4)+helps+support+coding+conventions+on+the+
team,+and+(5)+helps+improving+the+overall+process+and+quality+of+code.+”*
• Top*reason*for*44%*of*
managers.
• Top*reason*for*383*of*the*
programmers*(44%),*second*
motivation* for*204*(23%),*and*
third*for*96*(11%).*
20
Developer ’s*motivations*for*code*reviews.
21
Results:*Why*Do*Programmers*Do*Code*Reviews?
Results:*Why*Do*Programmers*Do*Code*Reviews?
B.'Code'Improvement
C.'Alternative'Solutions
• “Comments*or*changes*
about*code*in*terms*of*
readability,*commenting,*
consistency,*dead*code*
removal,*etc.,*but*do*not*
involve*correctness“.
• “Alternative+solutions+ regard*
changes*and*comments*on*
improving*the*submitted*
code*by*adopting*an*idea*
that*leads*to*a*better*
implementation“.
• Managers*reported*code+
improvement+as*their*primary*
motivation* in*51*(31%)*cases.
• Primary*motivation*for*337*
(39%)*programmers,*the*
second*for*208*(24%),*and*the*
third*for*135*(15%).*
Developer ’s*motivations*for*code*reviews.
• First*motivation*for*147*(17%)*
developers,* second*for*202*
(23%),*and*third*for*152*(17%)
• Only*4*(2%)*managers*even*
mentioned* it.
22
Results:*Why*Do*Programmers*Do*Code*Reviews?
23
Results:*Why*Do*Programmers*Do*Code*Reviews?
D.'Knowledge' Transfer
E.'Team'Awareness
• “If*you*do*a*code*review*and*
did*not*learn*anything*about*
the*area*…*then*that*was*not*
as*good*code*review*as*it*
could*have*been”.*
• “This*concept*of*using*an*
email*list*as*optional*reviewer,*
or*including*specific*optional*
reviewers*exclusively*for*
awareness”.
• Programmers:*first*motivation*
in*73*(8%)*cases,* second*in*
119*(14%),*and*third*in*141*
(16%).*
• Managers*included*
knowledge+transfer+as*one*of*
the*reasons*for*code*review,*
but*never*top.
Developer ’s*motivations*for*code*reviews.
Developer ’s*motivations*for*code*reviews.
24
• 873*programmers*answering*the*
survey*ranked*team+awareness+
and+transparency+very*close*to*
knowledge+transfer.*
• Programmers:*75*(9%)*
developers*considered*team*
awareness*their*first*motivation,*
108*(12%)*their*second,*and*149*
(17%)*their*third.*
Developer ’s*motivations*for*code*reviews.
25
Results:*Why*Do*Programmers*Do*Code*Reviews?
Results:*The*Outcomes*of*Code*Reviews
F.'Share'Code'Ownership
Question'2:'Outcomes'of'Code'Reviews
• “The*concept*of*shared+code+
ownership+is*closely*related*to*
team+awareness and*
transparency”.
• “Our*second*research*question*seeks*to*understand*what*the*actual*
outcomes*of*code*reviews*are,*and*whether*they*match*the*
motivations*and*expectations*outlined*in*the*previous*section.*“
• A*means*to*have*more*than*one*
knowledgeable*person*about*
specific*parts*of*the*codebase.*
• Improves*the*personal*
perception*of*team*members*
about*shared*code*ownership.*
• Programmers:*First*motivation*
for*51*(6%),*second*for*100*
(11%),*third*for*91*(10%).
• The*authors*analyzed*the*content*of*200*threads*(~570*comments)* from*
code*reviews.
• We*would*expect code*reviewers*to*talk*about*the*things* that*are*most*
important*to*them*(i.e.*that*outcome* would*match*motivations).
Developer ’s*motivations*for*code*reviews.
26
Results:*The*Outcomes*of*Code*Reviews
27
Discussion:* What*Are*The*Challenges*of*Code*Review?
When'Expectations'Do'Not'Meet'Reality
Question'3:'Challenges'
• “Our*third*research*question*seeks*to*understand*the*main*challenges*
faced*by*reviewers*when*performing*modern*code*reviews.*“
• “We*also*seek*to*uncover*the*reasons*behind*the*mismatch*between*
expectations*and*actual*outcomes*on*finding*defects*in*reviews.”
?
The*outcome*of*the*code*review*(reflected* in*code*comments)* does*not*match*the*
expected* focus*on*finding*defects.
28
29
Discussion:* What*Are*The*Challenges*of*Code*Review?
Discussion:* What*Are*The*Challenges*of*Code*Review?
Code'Review'is'Understanding
Code'Review'is'Understanding
• Interviews
• Understanding* was*identified* as*the*most*significant* challenge* when*
preparing*for*reviews.
• “The+most+difficult+thing+…+is+understanding+the+reason+of+the+change”.
• “Understanding+the+code+takes+most+of+the+reviewing+time.”*
• Code+Review+Comments
• Second*most*frequent*category*concerns*understanding* (i.e.*rationale*of*the*
changes,*corresponding*clarification* answers.*
• The*authors*included*a*question*in*the*programmers’*survey*to*know*
how+much+ understanding+ was+needed+to*achieve*each*motivation.
Result:* to*achieve*the*need*for*finding+defects,*context*and*understanding*the*
software*change*must*become*a*focus.
30
Discussion:* What*Are*The*Challenges*of*Code*Review?
31
Recommendations*and*Implications
The'Importance'of'Context
Recommendations'for'Practitioners
• Observation:*
• Implications*for*Developers
• Some*developers*thoroughly*read*the*code*review*description,*while*others*
navigated*directly*to*the*relevant*files*(i.e.*familiarity).
• “I+start+with+things+I+am+familiar+ with,+so+it+is+easier+to+see+ what+is+going+on.”*
• Programmers’*survey:
1. Whether*it*takes*longer*to*review*unfamiliar*files,*compared*to*files*that*you*know*
(91%*agree).
•
“When+reviewing+a+small,+unfamiliar+change,+it+is+often+necessary+to+read+through+ much+
more+code+than+that+being+reviewed.”*
2. Whether*reviewers*familiar*with*the*changed*files*give*different*feedback*(82%*
agree).
•
1. Quality*Assurance:*Mismatch* between* expectations* and*outcomes.*Don’t*
rely*on*code*reviews*to*identify*defects.
2. Beyond*Defects:* The*benefits* beyond*finding*defects* are*critical* (finding*
alternatives,* increase* learning,* share*code*ownership*etc.)*
3. Understanding:* Having*knowledge*of*the*context*and*code*helps*reviewers.*
Programmers*should*provide*context*and*direction*to*help*guide*reviews.
4. Communication:* Developers*need*richer*communication* than*annotated*
comments.* Teams* should*facilitate* inQperson*communication.
“It+takes+a+lot+longer+to+understand+ unknown+code,+but+even+then+understanding+ isn’t+
very+deep.+“
32
33
Recommendations*and*Implications
WrapQUp
Recommendations'for'Practitioners
Limitations
• Implications*for*Researchers
1. Research*conducted*strictly*within*one*company*has*limited*value*
or*applicability.
1. Automate* Code*Review*Tasks:*Automation* can*solve*“low*hanging*defects”*
and*code*improvements,* freeing*up*time*to*discover*more*subtle* defects.
2. Program*Comprehension* in*Practice:*Program*comprehension* tools*should*
be*leveraged;*we*can*do*better*than*diffs of*code*changes.
3. SocioQTechnical* Effects:* Awareness*and*learning*were*cited*as*motivations*
but*difficult*to*observe.*More*research*can*be*done*here.
• Is*Microsoft*representative?*
2. Some*motivations*are*not*observable*(e.g.*knowledge*transfer).
• Even*if*not*quantifiable,* these* motivations* are*“real”*and*need*to*be*
considered.
3. InQperson*discussions*were*observed*not*not*easily*codified.
34
WrapQUp
35
WrapQUp
Conclusions
Questions?
“Although+ the+top+motivation+ driving+code+reviews+is+finding+ defects,+
the+practice+and+the+actual+ outcomes+ are+less+about+ finding+errors+than+
expected…+code+review+provides+a+wide+spectrum+of+benefits+to+
software+teams,+such+as+knowledge+transfer,+team+awareness,+and+
improved+solutions+ to+problems”.
Are*you*convinced?
This*paper*contributes
• A*examination*of*the*underlying*motivations*for*modern*code*reviews
• The*observation*that*motivations*and*outcomes*that*don’t*always*match.*
• An*exploration*of*the*importance*of*understanding*in*effective* reviews.*
• Specific*recommendations*for*both*practitioners*and*researchers.*
36
37
WrapQUp
Introduction
Reference
Why'Code'Reviews?
1. Cohen, Jason. “Chapter 18: Modern Code Review” in Greg Wilson and Andy Oram. Making
Software: What Really Works, and Why We Believe It. New York. 2010.
2. Bacchelli, Alberto and Christian Bird. “Expectations, outcomes, and challenges of modern code
review”. Proceedings of the 2013 International Conference on Software Engineering (ICSE '13).
New York, 2013. 202-211.
3. Cohen, Jason. Four Ways To A Practical Code Review. http://www.methodsandtools.com.
Published Winter 2007. Retrieved Mar 13, 2015.
4. Dunsmore A., M. Roper and M. Wood. “Object-Oriented Inspection in the Face of Delocalisation”.
Proceedings of the 22nd International Conference on Software Engineering (ICSE) 2000: 467–476.
5. Cohen, Jason. Best Kept Secrets of Peer Code Review. Beverly, MA. SmartBear Software. 2006.
6. Votta, L. “Does every inspection need a meeting?” Proceedings of the 1st ACM SIGSOFT
Symposium on Foundations of Software Engineering, 1993: 107–114.
7. Fagan, M. Design and code inspections to reduce errors in program development. IBM Systems
Journal, 15 (3):182–211, 1976.
• Primary*goal:*
• Attempt*to**find*defects* ahead*of*
release.* (“Fix*it*while*it’s*cheap!”)
• Secondary*goals:
• Share*knowledge*of*the*codeQbase.
• Mentor*less*experienced* developers.
• Drastically*different*approaches*can*
be*taken*to*accomplish*these*goals
• Heavyweight*code*inspections* (Fagan)
• Lightweight* modern*code*review
Michael*Fagan:*Code*inspection*life*cycle*(1976)*[3].
38
Methodology: Data*Collection
39
Methodology:* Data*Collection
M1:'Analysis'of'Previous'Study
M2:'Interviews'&'Observations
• An*external*2012*study*investigated*how*different*product*teams*
were*using*CodeFlow.*It*consisted*of*structured*interviews*(lasting*30Q
50*minutes)*with*23*people*in*different*roles.*
• Interviews*&*Observations
• The*authors*used*grounded+ theory+to*derive*categories*of*questions,*
used*to*determine*four*motivations*for*code*review:
1. Finding*defects.
2. Maintaining*team*awareness.
3. Improving*code*quality.
4. Assessing*the*highQlevel*design.*
40
• OneQonQone*meetings* with*developers* who*use*CodeFlow (40Q60*minutes).*
• Interviewed:*five*developers,* four*senior*developers,* six*testers,* one*senior*
tester,*and*one*software*architect.*All*had*used*CodeFlow,*had*18*months*to*
almost*10*years*experience* (median*of*five*years).*
• Each*meeting* was*comprised* of*two*parts:*
1. Observing*them*perform*a*code*review.*
2. A*semiUstructured+followQup*interview.
• CodeFlow Comments
• Comments* were*extracted*and*categorized*from*the*code*review*sessions.
41
Methodology:* Data*Analysis
Methodology:* Validation
M3J5:'Processing'Feedback
M6:'Survey
• Card*Sorting
• Survey*1
• Used*to*group*categories* that*emerged*from*interviews/observations* and*
code*review*comments* extracted* from*CodeFlow.
• Open*card*sort:*groups*emerged*naturally*during*the*sorting*process.
• First*author*created*cards,*other*authors*created*categories,* performed*sort.
• Consensus* on*final*categories.
• Sent*to*a*cross*section* of*600*managers*that*had*at*least* 10*direct*or*indirect*
reporting*developers*who*used*CodeFlow.*
• The*central*focus*was*the*open*question* asking*to*enumerate* the*main*
motivations* for*doing*code*reviews*in*their*team.*
• They*received*165*answers*(28%*response* rate).
• Survey*2
• Affinity*Diagram
• 18*questions,* mostly*closed* with*multiple* choice*answers,*and*was*sent*to*
developers* who*signed* off*on*average*at*least* one*code*review*per*week.*
• They*received*873*answers*(44%*response* rate).*
• An*affinity+diagram+was+used+to+organize*the*categories* from*the*card*sort.*
42
43