= Review (NS-75): FR 1014, #2 review =
|| Author || Henrik Kirk ||
|| Moderator || Søren ||
|| State || Closed ||
== Objectives ==
{{{
Review recent changes to code to address errors in implementation of FR1014
}}}
== Summary ==
{{{
Review completed; Several severe errors found; as seen in the defects found.
Followup by HBK
}}}
'''General comments''':
|| '''Description''' || '''Classification''' || '''Status''' ||


=== Comments on file 'trunk/src/dk/netarkivet/harvester/scheduler/HarvestScheduler.java', revision 932 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 160-162 || Move this block inside method scheduleJobs, so it is run each time jobs is being scheduled || Cosmetic || OK ||
|| 191,191 || Missing '.' at end of first sentence || Cosmetic || OK ||
|| 202,202, 203,203 || The variable 'timediff' should be milliseconds, as Date.setTime() assumes the argument to be milliseconds. So you must need to multiply here with 1000. || Minor || OK ||
|| 209,209, 211,211 || Checkstyle: + at end of line must be moved to next line || Cosmetic || OK ||
|| 210,210 || divide by 60 to transform seconds to minutes || Cosmetic || OK ||
|| 217 || This line should be placed before the dao.update(job) command; otherwise the changes to job will not be persistent. || Minor || OK ||
|| 220 || Only write this log message, if stoppedJobs > 0 || Cosmetic || OK ||