= Review (NS-45): Review of FR 1628: Add custom JVM parameters to Heritrix subproces =
|| Author || Søren ||
|| Moderator || Søren ||
|| State || Closed ||
== Objectives ==
{{{
Review of files:
/trunk/src/dk/netarkivet/harvester/harvesting/JMXHeritrixController.java, revision 839, lines 251-291
/trunk/src/dk/netarkivet/harvester/HarvesterSettings.java, revision 839, lines 258-264
/trunk/src/dk/netarkivet/harvester/settings.xml, revision 839, lines 71
}}}
== Summary ==
{{{
follow-up by ngiraud
}}}
'''Total Time Used (Coding,Documentation,Review)''':

{{{
Time use (Coding,Documentation,Review)
NGiraud: 0.5
SVC: 0.5
}}}
'''General comments''':
|| '''Description''' || '''Classification''' || '''Status''' ||


=== Comments on file 'trunk/src/dk/netarkivet/harvester/HarvesterSettings.java', revision 839 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 261 || Add that "By default there is no additional JVM options" || Cosmetic || OK ||
|| 264 || Remove tab || Cosmetic || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/harvester/harvesting/JMXHeritrixController.java', revision 839 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| General || The tabs in many of the added should be replaced by spaces || Cosmetic || OK ||
|| 256 || Remove unused line || Cosmetic || OK ||
|| 289-290 || Move a local variable for allOpts.toArray(new String[allOpts.size())]], so we can fix feature request 1227 (Log the Heritrix command line) right now by including a log.info: "Starting Heritrix process with args" + args; || Cosmetic || OK ||