Hi Martin,

On Aug 15, 2012, at 2:26 AM, Martin Desruisseaux wrote:

> Hello all
> 
> Thanks Chris for all your comments. I got the acknowledgement from the Apache 
> secretary that the got my ICLA, so its look like that we can process. I 
> attached to this email I first patch proposal creating an initially empty 
> "sis-metadata" directory. The rest of this email is about question related to 
> this patch, the most important one probably being the GeoAPI dependency:

Got it, thanks!  Martin, can you please file an issue here:

https://issues.apache.org/jira/browse/SIS

And then attach your sis-metadata directory patch there? We use JIRA to 
track our issues and help to log our SVN commits and tie them to JIRA, so 
I'd appreciate you filing that issue and getting the credit for the patch.

> 
> 
> Missing "svn:ignore" (minor)
> ------------------------
> A quick note: isn't the "svn:ignore" property missing on the "sis-app" 
> directory? This property is presents in other directories (except "sis-data").

+1, another JIRA issue please :)

> 
> 
> Modules grouping (may be deferred)
> -------------------------------
> The attached patch creates the directory at the project root, because all 
> other SIS modules do that way. I'm not completely sure that it is worth to 
> group modules by categories; I did that way in Geotk mostly because GeoTools 
> did that way, and I originally wanted to stay close to GeoTools developers 
> habits. Whatever we should do that for Apache SIS or not is an open question.

Cool OK. Yeah for now, let's not group by categories, and if we find that we 
need to, that's fine too. 

> 
> 
> Parent pom.xml
> -------------
> I noticed that SIS put the parent pom.xml in a separated module, namely 
> "sis-parent". I wonder why the parent is not the root pom.xml? This would 
> reduce by 1 the amount of "modules". But surely I'm missing a reason?

This is a good question. We applied this procedure in Tika, and so I copied it 
through to SIS. I think that the rationale is that 
the parent pom contains simply properties that we wanted to be inherited (e.g., 
plugin configuration, build set up, etc.), but
it wasn't the multi-module project that would be ROOT/pom.xml as is currently. 
We don't expect any sis modules to inherit
that pom else they would inherit the multi-module definition. 

> 
> 
> <sis.version> property (minor)
> --------------------------
> I wonder also why declaring a "<sis.version>" property in the sis-parent 
> pom.xml given that the standard "$project.version" property fits the purpose?

+1, please file a JIRA issue for this and we'll convert it.

> 
> 
> GeoAPI dependency
> -----------------
> This patch modifies the sis-parent/pom.xml file for adding dependency 
> management items for GeoAPI 3.0.0 (for now, maybe we would depend on a 
> milestone later). In order to ensure version consistency, this patch does not 
> only manage GeoAPI dependencies. It also _import_ the GeoAPI 
> <dependencyManagement> section, using the Maven <scope>import</scope> 
> functionality [1]. This has the effect of importing three dependency 
> management items:
> 
>  * Units of measurement (currently JSR-275, but this project is dead...)

- do you know the license on this library or vecmath?

>  * vecmath
>  * JUnit
> 
> Because of the later, this import replaces the original JUnit management. 
> This has the side-effect of increasing the JUnit version from 3.8.1 to 4.8.2. 
> The reason why we import a JUnit dependency is because we inherit that 
> dependency from the geoapi-conformance module. geoapi-conformance "extends" 
> JUnit by providing validators and test methods for arbitrary implementations 
> of GeoAPI interfaces. Of course we have this dependency only in the tests, 
> not in the library itself.

+1 I'm fine with upgrading to a later Junit module. That's fine by me.

> 
> 
> What do peoples think?

Yep, others, please speak up :) From me, my comments are above and looking 
forward to the JIRA issues, and to getting this
going Martin. Thanks.

Cheers,
Chris

++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Chris Mattmann, Ph.D.
Senior Computer Scientist
NASA Jet Propulsion Laboratory Pasadena, CA 91109 USA
Office: 171-266B, Mailstop: 171-246
Email: [email protected]
WWW:   http://sunset.usc.edu/~mattmann/
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Adjunct Assistant Professor, Computer Science Department
University of Southern California, Los Angeles, CA 90089 USA
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

Reply via email to