Differences between revisions 1 and 4 (spanning 3 versions)
⇤ ← Revision 1 as of 2009-03-13 14:46:34 →
Size: 3922
Editor: SoerenCarlsen
Comment:
← Revision 4 as of 2010-08-16 10:24:30 → ⇥
Size: 3889
Editor: localhost
Comment: converted to 1.6 markup
Deletions are marked like this. Additions are marked like this.
Line 23: Line 23:
Line 31: Line 32:

Line 33: Line 36:
|| 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 || NOTOK ||
|| 222-227 || Fix this indentation. || Cosmetic || NOTOK ||
|| 141-143 || No Javadoc on this function. || Cosmetic || NOTOK ||		 || 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 ||
Line 38: Line 41:
|| 132 || "where to access" => the URL to a the GUI of running harvester || Cosmetic || NOTOK || || 132 || "where to access" => the URL to a the GUI of running harvester || Cosmetic || OK ||
Line 41: Line 44:
|| 47-51, 60-61 || Javadoc these constants. || Cosmetic || NOTOK ||
|| 46 || Missing dot in javadoc || Cosmetic || NOTOK ||
|| 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 || NOTOK ||
|| 286-291 || Javadoc this. || Cosmetic || NOTOK ||		 || 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 ||
Line 47: Line 50:
|| 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 || NOTOK ||
|| 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 || NOTOK ||
|| 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 || NOTOK ||
|| 507 || Remove empty line || Cosmetic || NOTOK ||		 || 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 ||
Line 53: Line 56:
|| 50-51 || Two empty lines next to each other. || Cosmetic || NOTOK || || 50-51 || Two empty lines next to each other. || Cosmetic || OK ||

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

IssuesFoundInReviewNs23 (last edited 2010-08-16 10:24:30 by localhost)