perlmeditation
Jazz
<P>Acting on the advice from [id://126106|this node], we have begun the planning process for basic code reviews on scripts listed at the <A HREF="http://www.perlarchive.com">Perl Archive</A>. This code review will aid in categorizing scripts based on its security and basic programming practices. I've used [id://67152|this node] as a reference when creating the basic points for code review.</P>
<P>Unless otherwise noted, each script will receive 1 point for compliance, -1 point for non-compliance on each of the following:</P>
<OL>
<LI>Uses warnings </LI>
<LI>Uses strict </LI>
<LI>Security (up to 4 points).
<UL>
<LI>Uses -T
<LI>Implements <I>valid</I> checks on all user input for potential security breaches or other damage</LI>
<LI>Does not appear to allow arbitrary commands
<LI>Using <A HREF="http://www.perldoc.com/perl5.6.1/lib/CGI.html#Avoiding-Denial-of-Service-Attacks">$CGI::POST_MAX</A> or otherwise limiting maximum post size (thanks [crazyinsomniac])</LI>
</UL>
<LI>Html output - uses [cpan://CGI], [cpan://HTML::Template], [cpan://HTML::Mason], or other suitable alternative</LI>
<LI>Form parsing - uses [cpan://CGI], [cpan://CGI::Lite], or other suitable <I>module-derived</I> alternative</LI>
<LI>Uses modules where applicable (-1 for using cgi-lib.pl) </LI>
<LI>Style, based on clarity and modularity (up to 2 points) </LI>
<LI>Documentation / comments (-2 for no comments; +2 for effective use of commenting) </LI>
<LI>Use HERE docs for lengthy text (-2 points for multiple print statements and "\"escape syndrome\"")</LI>
<LI>Checks return value of specific functions (aside from open, close, flock, can you suggest others to be added to this function list?)</LI>
<LI>Preserve file integrity by correctly using flock when necessary.</LI>
<LI><I>Anything else?</I></LI>
</OL>
<P>Since there will probably be instances when one or more of the points above will not be applicable to a script, a 0 point value will be used. This will equal N/A and will not affect the total score.</P>
<P>I realize that this does not come anywhere close to a comprehensive code review, but the only way we can realisticly implement any sort of code review at all is if we keep it simple (after all, there are ~4k scripts to review). Even this paltry review process can guilt/embarass even a few programmers into revamping their scripts, it will be worth the effort.</P>
<P>It's only fair that the program authors should bear the cost for more intensive code reviews on their own programs. If a program's author wishes to have an in-depth, individualized code review, we will refer them to various programmers who have expressed interest in performing this service (some for a fee, some as volunteers). The reviewer will then let me know the point score of the script. Perhaps there may be some rekindled interest in a [id://67152|code review section here]?</P>
<P>Once a script has been reviewed, it will have a "detail" page on the site with the results of the review. </P>
<P>Any suggestions, enhancements, or critiques you can offer on this list would be very helpful :)</P>
<P>Jasmine</P>
<P><B>Update:</B> Tainting/security point updated based on two replies from [wog] ([id://127213|1], [127215|2]) and a /msg from [crazyinsomniac].</P>
<P><B>Update:</B> Changed "excessive commenting" to "effective use of commenting", based on [rchiav]'s [id://127232|suggestion].</P>
<P><B>Update:</B> By [vroom|monk magic, I presume], this node has been relocated to Meditations, where I'm able to edit the root node (thanks!). So, the updated list is back here.