= Review (NS-29): Review of FR 1160 (Status and order selection (771) on History/Harveststatus-perharvestrun.jsp), and FR 1162 (Multiselect of status on all jobs) =
|| Author || Søren ||
|| Moderator || Søren ||
|| State || Closed ||
== Objectives ==
{{{
/trunk/src/dk/netarkivet/harvester/datamodel/JobDBDAO.java, r590, Lines 519-580
/trunk/src/dk/netarkivet/harvester/datamodel/JobDBDAO.java, r590, Lines 250-257
/trunk/src/dk/netarkivet/harvester/datamodel/JobStatus.java, r571, All lines
/trunk/webpages/History/Harveststatus-perharvestrun.jsp, r648,
/trunk/src/dk/netarkivet/harvester/webinterface/HarvestStatus.java
}}}
== Summary ==
{{{
Follow up by SVC
}}}
'''Total Time Used (Coding,Documentation,Review)''':

{{{
Time use (Coding,Documentation,Review)
SVC:  2.5
JOLF:  0.3
}}}
'''General comments''':
|| '''Description''' || '''Classification''' || '''Status''' ||


=== Comments on file 'trunk/webpages/History/Harveststatus-perhd.jsp', revision 714 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 170-202 || Change indentation (this is actually within a for-loop). || Cosmetic ||REJECTED ||
|| 213-215 || This section starts with "<%", but it never ends with a "%>". This is a Crucible bug -> end with empty line. || Cosmetic || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/harvester/datamodel/JobDBDAO.java', revision 590 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 546-548 || Though I can't see it for sure, due to the lacking lines, the 'sql' varialble seems to be a String. It should properly be a 'StringBuilder', since it has a lot of other strings appended. || Cosmetic || OK ||
|| 905 || Check with 'ArgumentNotValid' whether they are useable ('checkNotNegative' or 'checkPositive'). || Cosmetic ||OK ||
|| 905 || Javadoc this function. This is inherited, thus write 'See parent'. || Cosmetic || OK ||
|| 912-913 || Make Javadoc for the function. || Cosmetic || OK ||
|| 919 || "ASC" and "DESC" should be constants. || Cosmetic || OK ||
|| 941-942 || 'c' and 's' are bad names for a variables. || Cosmetic || OK ||
|| 962-970 || This is also used elsewhere. Thus make into a private method. || Cosmetic || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/harvester/webinterface/HarvestStatus.java', revision 714 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 224-225 || ArgumentNotValid.checkNotNegative på disse argumenter. || Cosmetic || OK ||
|| 236-238 || Log? or write comment about why not to log. || Cosmetic || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/harvester/datamodel/JobDAO.java', revision 590 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 252-256 || JAVADOC! || Cosmetic || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/harvester/datamodel/JobStatus.java', revision 571 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 105, 107, 109, 111, 113, 115, 117 || "status.job.new", "status.job.submitted", ... All these should be constants. || Cosmetic || OK ||
|| 127 || Argument not valid? || Cosmetic || OK ||
.