= Review (NS-98): FR #1660 Install script cannot handle the NetarchiveSuite.zip file having a different location =
|| Author || Jonas ||
|| Moderator || Jonas ||
|| State || Review ||
== Objectives ==
{{{
Make sure, that the solution is implemented correct:
Copies the NetarchiveSuite.zip file to the output directory.
Handles the following three scenarios:
1) The output directory is the same directory as the
NetarchiveSuite.zip file.
   Do nothing.
2) The output directory is different from the NetarchiveSuite.zip
file, but another NetarchiveSuite.zip file with the same name
is within the output directory.
   Do not copy file, but instead issue warning about the file
already existing.
3) The output directory is different from the NetarchiveSuite.zip
file and no other NetarchiveSuite.zip file with the same name
exists in the output directory.
   Copy the NetarchiveSuite.zip file to the output directory.
}}}
'''Total Time Used (Coding,Documentation,Review)''':

{{{
Time use (Coding,Documentation,Review)
KFC: 0.1
JOLF: 0.1
}}}
'''General comments''':
|| '''Description''' || '''Classification''' || '''Status''' ||


=== Comments on file 'trunk/src/dk/netarkivet/deploy/complete_settings.xml', revision 832 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| General || Move complete_settings to conf directory || Minor || Postpone ||
=== Comments on file 'trunk/src/dk/netarkivet/deploy/DeployConfiguration.java', revision 835 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 142-148 || Make into HTML || Cosmetic || OK ||
|| 167-169 || This seems a little strange to me. I give the program a NetarchiveSuite zip file, but another one is used, and it is in no way checked that they are identical. Imagine the case where you want to upgrade, and call deploy with a new NetarchiveSuite.zip, but the old one is used? || Minor || OK ||
|| 178 || Is it intentional that this will not print the stacktrace? Exception.toString will only print the exceptoins type and error message. That may be fine, in this case, though. Alternatively use e.printStackTrace(). || Cosmetic || OK ||
|| 179 || Shouldn't this be System.exit(1) or some other non-succes code? Generally change || Cosmetic || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/deploy/Constants.java', revision 835 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 466, 475 || Why are these warning identical? The first one is an error, why does it say 'warning'? Change the message in MSG_ERROR_ZIP_CANNOCIAL_PATH. || Cosmetic || OK ||