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