[Geoserver-devel] Re: Legend - code review (proposal to add Jin as a committer)

On Thursday 24 Jul 2003 10:06 pm, j.jin@anonymised.com wrote:

<snip>

ye, sure. ideally, if having the permission to use http cvs, i could commit
changes myself

Everyone:
Jin's code look quite good, I propose we add Jin as a committer to geotools.

Jin:
I've quickly looked at your Legend code but have not compiled and executed it.
Some suggestions for you:
1. I assume you have not compiled code using maven (since I see some netbeans
classes that are called).
Please ensure files compile with maven before committing to CVS. (Refer to
Developers Guide for how to do it). http://geotools.org/gt2docs/

2. Legend.java contains netbeans classes (put in by the Netbeans GUI
generator). They should be removed so geotools doesn't need to depend on
netbeans.

3. The GUI generator has put lots of "magic numbers" in your code. I think
this is because you are using an absolute layout. I suggest that
AbsoluteLayout should not be avoided if you can.

4. I would be good if you could write some unit tests for your code. (This is
hypocritical of me to say this since I have only written a couple of tests
for all my classes).

5. Please add javadocs for all your classes.

6. Run Jalopy (code formatter) over your code. Refer to developers guide
(netbeans section and jalopy section) on how to install and use it.

--
Cameron Shorter http://cameron.shorter.net
Open Source Developer http://generguide.sourceforge.net
                             http://mapbuilder.sourceforge.net
                             http://geotools.org
Senior Software Engineer http://www.adi-limited.com

Oops, sorry. I sent this to the wrong list. Too many geoXXX email addresses
in my autocomplete mailbox.

On Saturday 26 Jul 2003 7:14 am, Cameron Shorter wrote:

Everyone:
Jin's code look quite good, I propose we add Jin as a committer to
geotools.

--
Cameron Shorter http://cameron.shorter.net
Open Source Developer http://generguide.sourceforge.net
                             http://mapbuilder.sourceforge.net
                             http://geotools.org
Senior Software Engineer http://www.adi-limited.com