[Melbourne-pm] Do you run advanced lint scripts over your perl as part of a code review/release management process? (Was: Re: ISO 9126 and automated testing)

wigs at stirfried.org wigs at stirfried.org
Fri Jun 30 00:06:36 PDT 2006


On Fri, Jun 30, 2006 at 01:03:40PM +1000, Nathan Bailey wrote:
> I'd be interested to hear what people are doing in the 'automated code 
> review' space, i.e. where scripts and programs can identify bad logic or 
> poor use of syntax that is likely to lead to errors (simplest example is 
> the use of '=' for floating point numbers -- see Sakai URLs below for 
> more examples).

At work we don't have it automated at the point of doing a CVS commit, we do
the review as a manual step as part of the peer review process for every change
about to go into CVS.  We have a command line tool that started out as a
/bin/sh script but then quickly grew in complexity and was refactored into
perl.


For example, some code I was working on this afternoon went through the
process as follows:

> review lib/REA/Listing/Source.pm
File name:      /web/home/awigley/librea/lib/REA/Listing/Source.pm
MD5 Signature:  7e77c759d782a1e036649d92c1e08f3c
CVS Signature:  librea/lib/REA/Listing/Source.pm
Review ID:      27305

  Autodetect file type (Perl Module) .......................... PASS
  CVS Status is "Locally Modified" (working revision 1.6) ..... PASS
        Lines changed: -0 (0.0% changed) +17 (13.3% new)
  Tabs/Trailing whitespace .................................... PASS
  CVS Identification .......................................... PASS
  mod_perl safe code .......................................... PASS
  All TODO/XXX actions taken? ................................. PASS
  Hash keys unquoted .......................................... PASS
  Debugging harnesses removed ................................. PASS
  Block Indentation ........................................... PASS
  Documentation: readoc ....................................... PASS
  Valid readoc ................................................ PASS
  Quoted strings .............................................. PASS
  pragma: use strict; ......................................... PASS
  80 character line lengths ................................... PASS
  No private library paths .................................... PASS
  File Permissions ............................................ PASS
  Has $VERSION ................................................ PASS
  perl -cw -I~awigley/librea/lib:~awigley/librea/lib .......... PASS
  ~awigley/librea/lib/REA/Listing/Source.pm ready for peer-review

Suggestions:

 1. This file should have approval by XXX, YYY or ZZZ.

The following files are ready:
~librea/lib/REA/Listing/Source.pm


After the author runs the review program on their code where they have a chance
to correct their code based on the suggestions given.  The code reviewer then
repeats the process.  If the reviewer is then happy with the code they can do:

 > review approve $filename

Otherwise:

 > review reject $filename

The review process recognizes a number of different file types and runs
different tests as appropriate.  A history of the reviews and approvals
are store into a central database.

> I could imagine people incorporating some simple assertions into their 
> pre-commit cvs/svn scripts

This is something we have considered, and are likely to do in future.
Our review tool is not publically available however, and has many tests
idiomatic of our coding standards.

Anyone want a talk on this sometime?

-- 
Aaron



More information about the Melbourne-pm mailing list