[Geotools-devel] Catalog code review

[Jody G. <jga...@re...>]

Hi Justin,
You asked for a code review on your completing of the catalog interface...

I love your implementation of AdaptingResolve and so on:
- API Module - catalog patch accepted!
- Main - not quite ready yet

Cheers,
Jody

API Module
========
Code: +1
- looks nice and straight forward

Headers: +1
- I am module maintainer on this one and updated the header to reflect 
(C) 2006 (rather then (2002-2006 which you had). review.txt updated to 
reflect this.

javadocs: -0 (javadocs pass library standards, but we should do better 
for API module)
- class description
- method description
- For "core" modules like this that people have a chance of messing up I 
would like to include example code for at least ResolveAdapterFactory - 
I am thinking here of the poor GeoServer user who implemented canProcess 
as return true here (I know you do not expect people to read javadocs 
but I got to believe).
- package.html required

Tests: +0
- No test cases, expected for an interface only module

But yeah you pass your code review and I wont rollback your commit ;-)

Main Module
=========
org.geotools.catalog
--------------------
Code: +1
- Nice implementations

Headers: -1
- I suspect that things like DataStoreService has been created for you 
specifically for the GeoTools library (rather then backported)
- please see my email requesting details of what files here are 
backported vs created (I need to know before I can accept your code).

Tests: -1
- No test code :-(  This is not especially cool ... especially as 
initially your GeoTools friends will learn by reading your testcases
- you can relax test cases as more plugins provide you w/ coverage

org.geotools.catalog.adaptable
-------------------------------
Code: +1
- extra nice ;-) thanks man this is exactly what was missing

Headers: -1
- missing for some files

Tests: -1
- needed for implementation module

Javadocs: +0
- class and method descriptions
- more relaxed for an implementation
- package html, more relaxed for an implementation module