Review (NS-24): Review of FR 1485: File must be streamed instead on QA/QA-crawlloglines.jsp
Author |
Søren |
Moderator |
Søren |
State |
Closed |
Objectives
Files to review: QA/QA-crawlloglines.jsp, r536, lines 68-70 /trunk/src/dk/netarkivet/common/utils/StreamUtils.java, r562, lines 47-74 Note that StreamUtils r536 has been superseded by r562, that fixed a bug in the method
Summary
Review completed with JOLF and SVC as reviewers. SVC to followup
Total Time Used (Coding,Documentation,Review):
Time use (Coding,Documentation,Review) KFC: 0.5 SVC: 0.5 JOLF: 0.25
General comments:
Description |
Classification |
Status |
Comments on file 'trunk/webpages/QA/QA-crawlloglines.jsp', revision 536
Lines |
Description |
Classification |
Status |
52, 54 |
replace "jobid" and "domain" with constants. |
Cosmetic |
OK |
69 |
Remove outcommented code |
Cosmetic |
OK |
Comments on file 'trunk/src/dk/netarkivet/common/utils/StreamUtils.java', revision 562
Lines |
Description |
Classification |
Status |
65 |
Replace "UTF-8" with constant |
Cosmetic |
OK |
139 |
Replace "UTF-8" with constant |
Cosmetic |
OK |
71-72, 120-121, 149 |
If these IOFailures are not catched later and then logged, then they should be logged here. |
Cosmetic |
OK |
133-134 |
missing ArgumentNotValid validation. |
Cosmetic |
OK |