Jody and Andrea,

I have completed my IP review of gt-grassraster. Findings:

- All main source code files have GeoTools headers.

- JGrassUtilities.java (org.geotools.gce.grassraster.JGrassUtilities) 
looks like it contains colour tables (names and RGB components) that are 
directly converted from MIT X Window System's rgb.txt in its entirety. 
This should be acknowledged, and we should accept and include the 
relevant MIT license in LICENSE.txt(?) and/or review.apt.

- Format.java (org.geotools.gce.grassraster.core.color.Format) 
JlsTokenizer.java () has a GeoTools header, but it also has a 
third-party header. Should this have just the third-party header? If we 
have not substantially contributed to this file, then it should not have 
a a GeoTools header, as we can make no claim to it. The third-party 
header identifies the author as "The JODD team" and imposes what looks 
like a three-clause BSD licence (the good one). This needs to be 
included in LICENSE.txt(?) and/or review.apt.

- JlsTokenizer.java 
(org.geotools.gce.grassraster.core.color.JlsTokenizer) similarly has a 
third-party header that it may not need. It is by "JSkeet" and is 
identified as being LGPLv2 (or later). (Same as GeoTools, not further 
mention required.)

- Only one of the test classes has a GeoTools header. I like headers on 
tests, which are also code, but this is rarely enforced.

- What is the source of the data in TestMaps.java 
(org.geotools.gce.grassraster.TestMaps)? I see no mention.

- The test data files (not in test-data, I note) are pretty vague, but 
do not seem recognisable. Andrea, you might mention their source, if you 
know.

Good work, Andrea. Looks like, with a few minor review.apt updates, you 
get a big tick.

Kind regards,
Ben.


On 11/05/11 10:46, Ben Caradoc-Davies wrote:
> That's grassraster? Agreed.
>
> On 11/05/11 10:26, Jody Garnett wrote:
>> I will make you a deal; want to review the headers for moovida and I
>> will review for app-schema?
>

-- 
Ben Caradoc-Davies <[email protected]>
Software Engineering Team Leader
CSIRO Earth Science and Resource Engineering
Australian Resources Research Centre

------------------------------------------------------------------------------
Achieve unprecedented app performance and reliability
What every C/C++ and Fortran developer should know.
Learn how Intel has extended the reach of its next-generation tools
to help boost performance applications - inlcuding clusters.
http://p.sf.net/sfu/intel-dev2devmay
_______________________________________________
Geotools-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Reply via email to