Differences between revisions 1 and 5 (spanning 4 versions)

Review (NS-17): Bugfix 662

Author	Colin
Moderator	Colin
State	Closed

Objectives

To review the new utility for logging detailed causes on SQLExceptions. This method is called in each of the 73 places where we catch such exceptions.

Total Time Used (Coding,Documentation,Review):

Time use (Coding,Documentation,Review)
CSR: 2
SVC: 0.5

General comments:

Description	Classification	Status
Move '+' at the end of lines to the beginning of lines. This is a style violation	Cosmetic	OK

Comments on file 'trunk/src/dk/netarkivet/common/utils/ExceptionUtils.java', revision 650

Lines	Description	Classification	Status
66-74	I think this method breaks in line 69, if this argument e is null from the very start	Minor	OK

Comments on file 'trunk/src/dk/netarkivet/common/utils/ResultSetIterator.java', revision 650

NB. This abstract class is not in use. Documentation has been added but may be incorrect as there are no unit tests and the function is unclear.

Lines	Description	Classification	Status
82-84	Should add some javadoc. Could probably use the javadoc for FilterIterator.filter() as template: /** Returns the object corresponding to the given object, or null if * that object is to be skipped. * * @param o An object in the source iterator domain * @return An object in this iterators domain, or null */ protected abstract S filter(T o);	Cosmetic	POSTPONED
54-55	Add missing validation of res. Add ArgumentNotValid clause	Minor	OK

Comments on file 'trunk/src/dk/netarkivet/common/utils/DBUtils.java', revision 650

Lines	Description	Classification	Status
666	Too long line (> 80)	Minor	OK
392	Too long line	Cosmetic	OK
839	Too long line	Cosmetic	OK

Comments on file 'trunk/src/dk/netarkivet/harvester/datamodel/DerbyEmbeddedSpecifics.java', revision 650

Lines

Description

Classification

Status

Comments on file 'trunk/src/dk/netarkivet/harvester/datamodel/DBConnect.java', revision 650

Lines

Description

Classification

Status

-  ⇤ ← Revision 1 as of 2009-03-10 12:19:47 → 
  Size: 2285
  Editor: ColinRosenthal
  Comment:
+   ← Revision 5 as of 2010-08-16 10:25:15 → ⇥
  Size: 2429
  Editor: localhost
  Comment: converted to 1.6 markup
-Deletions are marked like this.
+Additions are marked like this.
 Line 4:
-|| State || Review ||
+|| State || Closed ||
 Line 10:
 {{{
-Line 12:
+Line 13:
 CSR: 2
-Line 15:
+Line 16:
 Line 18:
-|| Move '+' at the end of lines to the beginning of lines. This is a style violation || Cosmetic || NOTOK ||
+|| Move '+' at the end of lines to the beginning of lines. This is a style violation || Cosmetic || OK ||
 Line 23:
-|| 66-74 || I think this method breaks in line 69, if this argument e is null from the very start || Minor || NOTOK ||
+|| 66-74 || I think this method breaks in line 69, if this argument e is null from the very start || Minor || OK ||
 Line 25:
+'''''NB. This abstract class is not in use. Documentation has been added but may be incorrect as there are no unit tests and the function is unclear.'''''
-Line 26:
+Line 27:
-|| 82-84 || Should add some javadoc. Could probably use the javadoc for FilterIterator.filter() as template:  /** Returns the object corresponding to the given object, or null if    * that object is to be skipped.    *    * @param o An object in the source iterator domain    * @return An object in this iterators domain, or null    */     protected abstract S filter(T o); || Cosmetic || NOTOK ||
|| 54-55 || Add missing validation of res. Add ArgumentNotValid clause || Minor || NOTOK ||
+|| 82-84 || Should add some javadoc. Could probably use the javadoc for FilterIterator.filter() as template:  /** Returns the object corresponding to the given object, or null if    * that object is to be skipped.    *    * @param o An object in the source iterator domain    * @return An object in this iterators domain, or null    */     protected abstract S filter(T o); || Cosmetic || POSTPONED ||
|| 54-55 || Add missing validation of res. Add ArgumentNotValid clause || Minor || OK ||
-Line 30:
+Line 33:
-|| 666 || Too long line (> 80) || Minor || NOTOK ||
|| 392 || Too long line || Cosmetic || NOTOK ||
|| 839 || Too long line || Cosmetic || NOTOK ||
+|| 666 || Too long line (> 80) || Minor || OK ||
|| 392 || Too long line || Cosmetic || OK ||
|| 839 || Too long line || Cosmetic || OK ||