Hi Ben, thank you for your great effort. I will have a look at all your comments, check them out properly and come back to you.
Thanks again. Andrea On Tue, May 17, 2011 at 9:27 AM, Ben Caradoc-Davies <[email protected]> wrote: > 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
