= Review (NS-102): FR 1751: Create restart script for windows services =
|| Author || Jonas ||
|| Moderator || Jonas ||
|| State || Closed ||
== Objectives ==
{{{
}}}
'''Total Time Used (Coding,Documentation,Review)''':

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


=== Comments on file 'trunk/src/dk/netarkivet/deploy/Machine.java', revision 1050 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 827 || The Directory where there files will be placed. -- like above || Cosmetic || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/deploy/LinuxMachine.java', revision 1050 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 1068, 1104-1107 || Same as in WindowsMachine.  Bad handling of the error message, and undocumented exception thrown. || Cosmetic || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/deploy/WindowsMachine.java', revision 1050 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 1269, 1313 || It does not say anywhere, that it might throw an IOFailure. || Cosmetic || OK ||
|| 1310-1313 || Bad message handling.  The message it self should be a constant (in Contants like the others), and the error should not be part of the message string, it should be put along the message, e.g. log.trace(msg, e); throw new IOFailure(msg, e);  If the error is appended to the message string, it is the same as e.toString(), which only returns the name of the exception, not the cause or stacktrace. Why log.trace? This is ignored. Use log.warn or log.error instead. || Cosmetic || OK ||
|| 1318-1319 || I'll guess you mean 'Creates' || Cosmetic || OK ||
|| 1322, 1345 || It is also here not documented that it might throw an IOFailure. Remenber java-doc || Cosmetic || OK ||
|| 1342-1345 || Same as above. || Cosmetic || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/deploy/Constants.java', revision 1050 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 305 || 'restart' || Cosmetic || OK ||
|| 307 || 'wait' || Cosmetic || OK ||
|| 309 || 'kill_.*' || Cosmetic || OK ||
|| 311 || 'start_.*' || Cosmetic || OK ||