Why do we need to do a code review? Code reviews are an essential component of software quality assurance. A code review involves having some third party developer take a look at the code and look for errors. It can be a developer internal to your organization or someone external you can trust. A typical review consists of a line-by-line analysis of the source code of a project. Typically things looked for include:
- Removal of dead or unreachable code
- Verification of adherence to organization-wide coding standards (includes labeling of code, asset tracking flags, check-in to source code control systems, etc.)
- Performance check
- Security check
Oddly enough the way most companies handle internal code reviews should be modified when dealing with open source code. Most organizations have developers read the source files in their favorite editor, eyeball the code, and verbally certify that it meets standards. They do this without much in the way of documentation and this simply will not fly on a DIY LIMS project since much of the source is coming from outside of your organization.
What you need is a quarantine method where all new code coming in from the wild is processed, scanned, and then certified for internal use. Lots of open source projects rely upon the developer community to do this but scientific/laboratory software often does not get this kind of perusal. Your best bet is to arrange for the code review yourself.
The product of a good code review will come in the form of one of two types of documents depending on what type of release the software is in. The first is a full code review document and the second is for a minor release.
Before you can begin you'll need to be aware of the differences in naming conventions for software releases. Having a defined method for differentiating a major or minor release has a direct impact on how much work it will take for wild code to escape the gravity of the code quarantine chamber. Different projects have different version naming conventions so read the release notes to figure out what scheme the developers are using.
The first type of a full code review document (or it can be a set of documents, but if there is more than one there must be a master document that references the others as sub-documents) should contain ALL of the source code files in the release -- anything with executable code. So, if the program was written in Java it will contain all of the x.java files and anything that may contain executable code.
If you are using Linux (OpenSUSE example is shown here) you can use a script to grab all of the main Java files in a directory and pound them into a PDF like this (uses Perl, enscript, and ps2pdf):
find $1 -name '*.java' -print > sources.txt
echo 'Creating allsource.txt...'
perl -ne 'print qq|$_ | . `cat -n $_`' sources.txt > allsource.txt
enscript allsource.txt -o icrisatlims_3.0.0_source.ps
echo 'Converting to PDF...'
echo 'Process complete.'
For an example of this output refer to the Docs page at this site.
This, provided there is no other executable code in the release, is what you will need for a full release code review. For this project the release number, 3.0.0, indicates a major release. If it was some form of minor release you'll need to look for three things: files that are no longer in the release, new files, and finally changes to existing files. You could perform a full code review for each and every release but that is wasteful since files that have not changed do not need to be reviewed again.
Once you have your code review document the final step is to have the reviewer sign each and every page. You'll want to have perused the code reviewer's credentials beforehand to ensure that the individual is familiar with the programming language and the system type.
Code reviewing is very different from testing the software. During testing the developers are looking for actual run-time errors, data inconsistencies, bugs and the like. In code reviews the reviewer is looking for potentially dangerous things like infinite loops, memory leaks, and/or developer leftovers that may be usable as potential backdoors. For example, in the previously referenced PDF, take a look at page 42, line 84. Here it looks like the developers 'Eashwar' and 'Punna Ramu' elected to immortalize themselves in the source code. Looking for things like this is part and parcel of the code review process.Go Back
Citation: Open Source Code Reviews. (2012). Retrieved Sun Apr 30 05:01:36 2017, from http://www.limsexpert.com/cgi-bin/bixchange/bixchange.cgi?pom=limsexpert3;iid=readMore;go=1352569317