You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sis.apache.org by Martin Desruisseaux <ma...@geomatys.fr> on 2013/08/27 17:02:44 UTC

Shapefile on development branch, proposed next steps

Hello Travis

I performed "svn copy storage/sis-shapefile" from the Shapefile branch 
to the JDK7 branch. Before to continue, would you agree if I perform the 
following changes?

  * Apply a uniform Apache License 2.0 header on every files.
  * Replace tabulations by spaces.
  * Replace usage of Feature by DefaultFeature.
  * Move the "data" directory to
    "src/test/resources/org/apache/sis/storage/shapefile"
    ("src/test/resources" is the standard Maven directory for test files).
  * Move the "org.apache.sis.storage.shapefile.test" package one level
    up (to "org.apache.sis.storage.shapefile", i.e. together with the
    classes to be tested). This allows access to package-privated members.


That should quick. Next, I would merge the JDK7 branch to the trunk.

On which branch do you prefer to work, JDK7 or trunk? If you can work 
with JDK7, I could test and make proposals a little bit quicker, but 
trunk is fine as well - at your choice.

     Martin


Re: Shapefile on development branch, proposed next steps

Posted by Adam Estrada <es...@gmail.com>.
Thanks a bunch, guys! This looks like it's coming along nicely!

Adam


On Thu, Aug 29, 2013 at 10:35 AM, Travis L Pinney
<tr...@gmail.com>wrote:

> That works. I will make that change.
>
> Cheers,
> Travis
>
>
>
> On Thu, Aug 29, 2013 at 10:30 AM, Martin Desruisseaux <
> martin.desruisseaux@geomatys.fr> wrote:
>
> > Hello Travis
> >
> > Le 29/08/13 16:15, Travis L Pinney a écrit :
> >
> >  FieldDescriptor is specific to the ShapeFile internal format. When you
> >> say packaged private do you mean a private inner class with another
> >> class?
> >>
> >
> > Just removing the "public" keyword before "class". That way, the class
> > become visible only to other classes in the same package.
> >
> >     Thanks,
> >
> >         Martin
> >
> >
>

Re: Shapefile on development branch, proposed next steps

Posted by Travis L Pinney <tr...@gmail.com>.
That works. I will make that change.

Cheers,
Travis



On Thu, Aug 29, 2013 at 10:30 AM, Martin Desruisseaux <
martin.desruisseaux@geomatys.fr> wrote:

> Hello Travis
>
> Le 29/08/13 16:15, Travis L Pinney a écrit :
>
>  FieldDescriptor is specific to the ShapeFile internal format. When you
>> say packaged private do you mean a private inner class with another
>> class?
>>
>
> Just removing the "public" keyword before "class". That way, the class
> become visible only to other classes in the same package.
>
>     Thanks,
>
>         Martin
>
>

Re: Shapefile on development branch, proposed next steps

Posted by Martin Desruisseaux <ma...@geomatys.fr>.
Hello Travis

Le 29/08/13 16:15, Travis L Pinney a écrit :
> FieldDescriptor is specific to the ShapeFile internal format. When you
> say packaged private do you mean a private inner class with another
> class?

Just removing the "public" keyword before "class". That way, the class 
become visible only to other classes in the same package.

     Thanks,

         Martin


Re: Shapefile on development branch, proposed next steps

Posted by Travis L Pinney <tr...@gmail.com>.
Hi Martin,

Regarding making the fields private, I need to research that more. It
will be good to keep api stability for future evolutions as well as
keep the code simple. I am not opposed to creating getter-setter
functions.

There is a relationship between CodePage and java.nio.charset.Charset,
I am not sure if everything maps exactly but it would be good to
leverage it.

https://www.iana.org/assignments/character-sets/character-sets.xhtml

+1 on the "Simple Feature specification" and making it more generic.

FieldDescriptor is specific to the ShapeFile internal format. When you
say packaged private do you mean a private inner class with another
class?

I will look into making the CodePage more integrated with how java
handles Charsets first

Thanks!
Travis







On Wed, Aug 28, 2013 at 5:58 PM, Martin Desruisseaux
<ma...@geomatys.fr> wrote:
> Hello Travis
>
> There is a few though that popup from my head:
>
>  * I noticed that in the ShapeFile class, all fields are public. Do you
>    think we could make them private in order to keep more flexibility
>    for future evolutions?
>  * I wonder if there is some relationship planed between CodePage and
>    java.nio.charset.Charset?
>  * The ShapeTypeEnum seems to contain well known types from "Simple
>    Feature" specification. I wonder if we could / should handle them in
>    a way that apply to a wider range of storage? (Johann could tell
>    more on this topic).
>  * FieldDescriptor seems quite specific to the ShapeFile internal.
>    Should it be package-privated?
>
>
>     Cheers,
>     Martin
>

Re: Shapefile on development branch, proposed next steps

Posted by Martin Desruisseaux <ma...@geomatys.fr>.
Hello Travis

There is a few though that popup from my head:

  * I noticed that in the ShapeFile class, all fields are public. Do you
    think we could make them private in order to keep more flexibility
    for future evolutions?
  * I wonder if there is some relationship planed between CodePage and
    java.nio.charset.Charset?
  * The ShapeTypeEnum seems to contain well known types from "Simple
    Feature" specification. I wonder if we could / should handle them in
    a way that apply to a wider range of storage? (Johann could tell
    more on this topic).
  * FieldDescriptor seems quite specific to the ShapeFile internal.
    Should it be package-privated?


     Cheers,
     Martin


Re: Shapefile on development branch, proposed next steps

Posted by Travis L Pinney <tr...@gmail.com>.
Thanks!


On Tue, Aug 27, 2013 at 1:57 PM, Martin Desruisseaux
<ma...@geomatys.fr> wrote:
> Le 27/08/13 17:35, Travis L Pinney a écrit :
>>
>> +1 for everything.
>
>
> Thanks, done and merged to trunk.
>
>
>> I can work with JDK7 if that makes things easier.
>
>
> I think that would be easier, if you don't mind... I will post some
> proposals tomorrow.
>
>     Cheers
>     Martin
>

Re: Shapefile on development branch, proposed next steps

Posted by Martin Desruisseaux <ma...@geomatys.fr>.
Le 27/08/13 17:35, Travis L Pinney a écrit :
> +1 for everything.

Thanks, done and merged to trunk.

> I can work with JDK7 if that makes things easier.

I think that would be easier, if you don't mind... I will post some 
proposals tomorrow.

     Cheers
     Martin


Re: Shapefile on development branch, proposed next steps

Posted by Travis L Pinney <tr...@gmail.com>.
Hi Martin,

+1 for everything.

I can work with JDK7 if that makes things easier.

Thanks!
Travis


On Tue, Aug 27, 2013 at 11:02 AM, Martin Desruisseaux
<ma...@geomatys.fr> wrote:
> Hello Travis
>
> I performed "svn copy storage/sis-shapefile" from the Shapefile branch to
> the JDK7 branch. Before to continue, would you agree if I perform the
> following changes?
>
>  * Apply a uniform Apache License 2.0 header on every files.
>  * Replace tabulations by spaces.
>  * Replace usage of Feature by DefaultFeature.
>  * Move the "data" directory to
>    "src/test/resources/org/apache/sis/storage/shapefile"
>    ("src/test/resources" is the standard Maven directory for test files).
>  * Move the "org.apache.sis.storage.shapefile.test" package one level
>    up (to "org.apache.sis.storage.shapefile", i.e. together with the
>    classes to be tested). This allows access to package-privated members.
>
>
> That should quick. Next, I would merge the JDK7 branch to the trunk.
>
> On which branch do you prefer to work, JDK7 or trunk? If you can work with
> JDK7, I could test and make proposals a little bit quicker, but trunk is
> fine as well - at your choice.
>
>     Martin
>