Review (NS-23): FR 1497 (Button in NetarchiveSuite)

Author	Søren
Moderator	Søren
State	Closed

Objectives

Review of code that fixed FR 1497:
   trunk/src/dk/netarkivet/harvester/harvesting/DirectHeritrixController.java, r537, lines 193-201
   trunk/src/dk/netarkivet/harvester/harvesting/HeritrixController.java,  r537, lines 131-135
   trunk/src/dk/netarkivet/harvester/harvesting/HeritrixLauncher.java,  r537, lines 216-219
   trunk/src/dk/netarkivet/harvester/harvesting/JMXHeritrixController.java,  r537, lines  501-508
   trunk/src/dk/netarkivet/monitor/webinterface/JMXSummaryUtils.java,  r537, lines 234-235, 242
The primary changes:
Addition of getHarvestInformation() method to HeritrixController interface, and its implementation in DirectHeritrixController and JMXHeritrixController.
Use this method in a logmessage in HeritrixLauncher.doCrawlLoop()
In JMXSummaryUtils.generateMessage, any text found in a logmessage that starts with http are turned into a clickable link.

Summary

SVC to follow-up

Total Time Used (Coding,Documentation,Review):

Time use (Coding,Documentation,Review)
KFC: 1
SVC: 0.25
JOLF: 0.25

General comments:

Description

Classification

Status

Comments on file 'trunk/src/dk/netarkivet/harvester/harvesting/DirectHeritrixController.java', revision 537

Lines	Description	Classification	Status
193-200	What if the harvester is not running?? It will still return "running inline", even if does not run. Also shouldn't "running inline" be a put into a constant? Followup: Add a TODO to make this method respond after how the Harvester really is doing, and not just respond "Running inline"	Cosmetic	OK
222-227	Fix this indentation.	Cosmetic	OK
141-143	No Javadoc on this function.	Cosmetic	OK

Comments on file 'trunk/src/dk/netarkivet/harvester/harvesting/HeritrixController.java', revision 537

Lines	Description	Classification	Status
132	"where to access" => the URL to a the GUI of running harvester	Cosmetic	OK

Comments on file 'trunk/src/dk/netarkivet/monitor/webinterface/JMXSummaryUtils.java', revision 537

Lines	Description	Classification	Status
47-51, 60-61	Javadoc these constants.	Cosmetic	OK
46	Missing dot in javadoc	Cosmetic	OK
235	This line is too long. Add comment to this line: All strings starting with "http:" or "https:" are replaced with proper HTML Anchor	Cosmetic	OK
286-291	Javadoc this.	Cosmetic	OK

Comments on file 'trunk/src/dk/netarkivet/harvester/harvesting/JMXHeritrixController.java', revision 537

Lines	Description	Classification	Status
252-255, 288-291, 296, 301-303, 308, 318, 325-327, 346, 356, 390, 454	These function does something else than just call the HeritrixController#cleanup() function. What? and why not describe it?	Cosmetic	OK
71, 78-86	Javadoc these constants and the log. And add TODO: Add feature request in the Heritrix project to make publicly available constants for these attributes in the Heritrix code.	Cosmetic	OK
157-165	Also Javadoc these constants. Why are these constants not publicly available in the Heritrix code. Add feature request in the Heritrix project	Cosmetic	OK
507	Remove empty line	Cosmetic	OK

Comments on file 'trunk/src/dk/netarkivet/harvester/harvesting/HeritrixLauncher.java', revision 537

Lines	Description	Classification	Status
50-51	Two empty lines next to each other.	Cosmetic	OK