[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