NetarchiveSuite
Review (NS-60): FR 1709
Author |
ONB |
Moderator |
Søren |
State |
Closed |
Objectives
Summary
Andreas has already acted on most of the defects reported by Henrik and I (Søren). The remaining followup needed will be done by Andreas and/or Søren
General comments:
Description |
Classification |
Status |
Comments on file 'trunk/src/dk/netarkivet/common/utils/FreeSpaceProviderFactory.java', revision 892
Lines |
Description |
Classification |
Status |
General |
Missing subversion keys |
Cosmetic |
OK |
29-30 |
Wrong class javadoc |
Cosmetic |
OK |
Comments on file 'trunk/src/dk/netarkivet/common/utils/DefaultFreeSpaceProvider.java', revision 892
Lines |
Description |
Classification |
Status |
General |
Missing subversion keys |
Cosmetic |
OK |
50 |
File f |
Cosmetic |
OK |
Comments on file 'trunk/tests/dk/netarkivet/archive/bitarchive/IntegrityTests.java', revision 892
Lines |
Description |
Classification |
Status |
119 |
. |
Cosmetic |
OK |
138 |
. |
Cosmetic |
OK |
152 |
. |
Cosmetic |
OK |
190 |
. |
Cosmetic |
OK |
Comments on file 'trunk/src/dk/netarkivet/common/utils/FreeSpaceProvider.java', revision 892
Lines |
Description |
Classification |
Status |
General |
Missing subversion keys |
Cosmetic |
OK |
General |
Why not change this abstract class to an interface |
Cosmetic |
OK |
Comments on file 'trunk/src/dk/netarkivet/common/utils/FileUtils.java', revision 892
Lines |
Description |
Classification |
Status |
686, 690 |
FreeSpaceCalculation class => FreeSpaceProvider class defined by the setting CommonSettings.FREE_SPACE_PROVIDER_CLASS (a.k.a settings.common.freespaceprovider.class) |
Cosmetic |
OK |
690 |
defined in settinds.xml. |
Cosmetic |
OK |
Comments on file 'trunk/src/dk/netarkivet/common/utils/FileFreeSpaceProviderSettings.xml', revision 892
Lines |
Description |
Classification |
Status |
General |
Missing subversion keys |
Cosmetic |
OK |
Comments on file 'trunk/src/dk/netarkivet/common/CommonSettings.java', revision 892
Lines |
Description |
Classification |
Status |
348-349 |
... utils.FreeSpaceProvider |
Cosmetic |
OK |
350 |
Change to FREESPACE_PROVIDER_CLASS |
Cosmetic |
OK |
Comments on file 'trunk/src/dk/netarkivet/common/utils/FileFreeSpaceProvider.java', revision 892
Lines |
Description |
Classification |
Status |
General |
Missing subversion keys |
Cosmetic |
OK |
40 |
Should be renamed FilebasedFreeSpaceProvider |
Cosmetic |
OK |
74 |
name as parameter f. |
Cosmetic |
OK |
81 |
Second parameter: "File f" |
Cosmetic |
OK |
92 |
While loop overwrites bytes for every line, is this the wanted result, shouldn't it either bytes += Long.parseLong() or only read one line? |
Minor |
OK |
Comments on file 'trunk/src/dk/netarkivet/common/utils/MockFreeSpaceProvider.java', revision 892
Lines |
Description |
Classification |
Status |
General |
Missing subversion keys |
Cosmetic |
OK |
General |
Move to the tests/dk/netarkivet/common/utils/ |
Cosmetic |
OK |