Differences between revisions 1 and 3 (spanning 2 versions)

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

-  ⇤ ← Revision 1 as of 2009-10-21 11:19:50 → 
  Size: 2358
  Editor: JonasFrellesen
  Comment:
+   ← Revision 3 as of 2010-08-16 10:24:49 → ⇥
  Size: 2322
  Editor: localhost
  Comment: converted to 1.6 markup
-Deletions are marked like this.
+Additions are marked like this.
 Line 7:
-Line 10:
+Line 9:
 {{{
 Line 15:
-Line 22:
+Line 21:
-|| 827 || The Directory where there files will be placed. -- like above || Cosmetic || NOTOK ||
+|| 827 || The Directory where there files will be placed. -- like above || Cosmetic || OK ||
-Line 25:
+Line 24:
-|| 1068, 1104-1107 || Same as in WindowsMachine.  Bad handling of the error message, and undocumented exception thrown. || Cosmetic || NOTOK ||
+|| 1068, 1104-1107 || Same as in WindowsMachine.  Bad handling of the error message, and undocumented exception thrown. || Cosmetic || OK ||
-Line 28:
+Line 27:
-|| 1269, 1313 || It does not say anywhere, that it might throw an IOFailure. || Cosmetic || NOTOK ||
|| 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 || NOTOK ||
|| 1318-1319 || I'll guess you mean 'Creates' || Cosmetic || NOTOK ||
|| 1322, 1345 || It is also here not documented that it might throw an IOFailure. Remenber java-doc || Cosmetic || NOTOK ||
|| 1342-1345 || Same as above. || Cosmetic || NOTOK ||
+|| 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 ||
-Line 35:
+Line 34:
-|| 305 || 'restart' || Cosmetic || NOTOK ||
|| 307 || 'wait' || Cosmetic || NOTOK ||
|| 309 || 'kill_.*' || Cosmetic || NOTOK ||
|| 311 || 'start_.*' || Cosmetic || NOTOK ||
+|| 305 || 'restart' || Cosmetic || OK ||
|| 307 || 'wait' || Cosmetic || OK ||
|| 309 || 'kill_.*' || Cosmetic || OK ||
|| 311 || 'start_.*' || Cosmetic || OK ||