[SoC] GDAL Code Reviews - SoC and beyond

Frank Warmerdam warmerdam at pobox.com
Tue Jul 3 10:21:46 EDT 2007


Folks,

A decade ago when I worked in an office, we had developed a practice of
regular code reviews.  These were, I found, an excellent educational
tool for sharing best practice, and also helpful in identifying problems
in the reviewed code.

As part of our commitment to our Summer of Code students, I'd like to
introduce this practice to GDAL.  While my initial aim is to give our
SoC students an opportunity to review other code, and to be reviewed,
I also am hopeful that we could continue the practice after this point.

My concept of code reviews is that we would:

  o Pick a particular module of code, hopefully around 1000 lines of
    code and not more than 2000.

  o Get four reviewers to spend an hour each pre-reviewing the code, and
    making notes on points for discussion.

  o Getting together in IRC, following along "line by line", likely using
    a line numbered view like:
     http://trac.osgeo.org/gdal/browser/trunk/gdal/port/cpl_minixml.cpp

  o Spend about 1 hour reviewing in IRC, lead by a lead reviewer who
    keeps things moving.

  o The Reviewee would be responsible for recording action items.

  o The Reviewee would be expected to act on the suggestions, making the
    updated version available (along with the action items) within one week
    of the review for offline followup by reviewers.

One challenge for code reviews is getting reviewers willing to commit the
two hours needed for the process.  I would suggest that we have our paid
maintainer (Mateusz) as one reviewer.  That for the SoC reviews we include
at on other SoC student as a reviewer, and that the mentors have to review
at least their own mentee.  That should give us 3 of 4 reviewers.  We will
call for volunteers for the fourth spot.

Any opinions on this idea?  Are there any SoC students who can't can't access
IRC?

Are there any SoC students who will have code available for review reasonably
soon?    I would like to suggest that we pick some other piece of code
initially for review.  As I would like to lead the first review, it should be
someone elses code.  I'll ask around a bit for some recently contributed code
by another author which can be the reviewed module.

Best regards,
-- 
---------------------------------------+--------------------------------------
I set the clouds in motion - turn up   | Frank Warmerdam, warmerdam at pobox.com
light and sound - activate the windows | http://pobox.com/~warmerdam
and watch the world go round - Rush    | President OSGeo, http://osgeo.org



More information about the Soc mailing list