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 ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys -- and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV _______________________________________________ Geotools-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/geotools-devel
