= Review (NS-31): Review of bugfix for bug 1564: Disk mount filled to last block - and then it's not possible to move a single file =
|| Author || Søren ||
|| Moderator || Søren ||
|| State || Closed ||
== Objectives ==
{{{
Review of bugfix for bug 1564.
Review of the fixed createBitarchiveAppId(). Now uses the CommonSettings.APPLICATION_INSTANCE_ID setting (if set), instead of CommonSettings.HTTP_PORT_NUMBER to distinguish between multiple apps on the same machine
Review minor fixes to build.xml (added derbytools library to the classpath written to the MANIFEST.MF),
ArgumentNotValid (corrected method modifier order), GetDataResolver(better logging), FileUtils (better error-checking), and DomainIngester (javadoc added)
BitarchiveAdmin, r752, Lines 136-177
BitarchiveServer, r752, Lines 455-500
SingleMBeanObject, r770, Lines 193,198, 219, 222-228
Bitarchive, r752, Lines 220-221
build.xml, r762, Lines 35
ArgumentNotValid, r763, Lines 27-164
FileUtils, r770, Lines 303, 309-312, 419-420, 427-431, 528-529, 533-536, 958-959, 964-967, 987, 992-995
GetDataResolver, r770, Lines 168-169, 199-200, 233-234
}}}
== Summary ==
{{{
Follow up by SVC
}}}
'''Total Time Used (Coding,Documentation,Review)''':

{{{
Time use (Coding,Documentation,Review)
SVC: 1.5 md
JOLF: 0.5 md
}}}
'''General comments''':
|| '''Description''' || '''Classification''' || '''Status''' ||


=== Comments on file 'trunk/src/dk/netarkivet/common/management/SingleMBeanObject.java', revision 770 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
=== Comments on file 'trunk/src/dk/netarkivet/viewerproxy/GetDataResolver.java', revision 770 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 168-170 || log? || Cosmetic || OK ||
|| 199-200 || log? || Cosmetic || OK ||
|| 233-234 || log? || Cosmetic || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/archive/bitarchive/Bitarchive.java', revision 752 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 218 || Make ArgumentNotValid - check on FileBatchJob. || Cosmetic || OK ||
=== Comments on file 'trunk/build.xml', revision 762 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
=== Comments on file 'trunk/src/dk/netarkivet/harvester/webinterface/DomainIngester.java', revision 764 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 40 || Should this be public or private? || Cosmetic || OK ||
|| 42, 44 || Are these variables supposed to be public? || Cosmetic || OK ||
|| 59 || Check arguments || Cosmetic || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/archive/bitarchive/BitarchiveAdmin.java', revision 752 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 153 || The variablename "i" is usually used as an integer for going through the "index" in for-loops.  Could be replaced with "iterator" or just "it". || Cosmetic || OK ||
|| 156-159 || Implement the wanted feature in the TODO. This is already a feature request. Refer to this in the comments. || Cosmetic || OK ||
|| 160-161 || Slå disse if-statements sammen til: if(checkArchiveDir(dir) && (bytesFreeInDir > minSpaceLeft + requestedSize)) || Minor || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/common/utils/FileUtils.java', revision 770 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 310-311 || Log? || Cosmetic || OK ||
|| 428-429 || log? || Cosmetic || OK ||
|| 534-535 || log? || Cosmetic || OK ||
|| 965-966 || log? || Cosmetic || OK ||
|| 993-994 || log? || Cosmetic || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/common/exceptions/ArgumentNotValid.java', revision 763 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 61 || use val.isEmpty() || Cosmetic || OK ||
|| 74 || Wrong order => public static void || Cosmetic || OK ||
|| 88, 102 || Why use 'i' when standard integer and 'num' when long integer? || Cosmetic || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/archive/bitarchive/distribute/BitarchiveServer.java', revision 752 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||