Re: [Geoserver-devel] review of csw pull request

2013-02-12 Thread Niels Charlier
Hi Justin, Sorry for the late response, lost track of this a bit. Rather than being needlessly complicated I believe I have caused confusion by giving these two methods the same name - I should not have done that. What they have in common is that they both convert from Name to P

Re: [Geoserver-devel] review of csw pull request

2013-02-07 Thread Justin Deoliveira
Thanks for the explanation Niels. Some comments/questions inline. On Thu, Feb 7, 2013 at 6:18 AM, Niels Charlier wrote: > On 02/07/2013 02:02 PM, Andrea Aime wrote: > > On Thu, Feb 7, 2013 at 1:57 PM, Niels Charlier wrote: > >> On 02/06/2013 06:22 PM, Andrea Aime wrote: >> >>> >>> The idea tha

Re: [Geoserver-devel] review of csw pull request

2013-02-07 Thread Niels Charlier
On 02/07/2013 02:02 PM, Andrea Aime wrote: On Thu, Feb 7, 2013 at 1:57 PM, Niels Charlier > wrote: On 02/06/2013 06:22 PM, Andrea Aime wrote: The idea that there are two separate ways to translate between the internal data model and the expos

Re: [Geoserver-devel] review of csw pull request

2013-02-07 Thread Andrea Aime
On Thu, Feb 7, 2013 at 1:57 PM, Niels Charlier wrote: > On 02/06/2013 06:22 PM, Andrea Aime wrote: > >> >> The idea that there are two separate ways to translate between the >> internal data model and >> the exposed data model bugs me, seems redundant and needlessly >> complicated. >> >> > How do

Re: [Geoserver-devel] review of csw pull request

2013-02-07 Thread Niels Charlier
On 02/06/2013 06:22 PM, Andrea Aime wrote: > > The idea that there are two separate ways to translate between the > internal data model and > the exposed data model bugs me, seems redundant and needlessly > complicated. > How do you mean there are two separate ways to translate between internal

Re: [Geoserver-devel] review of csw pull request

2013-02-06 Thread Justin Deoliveira
Ok cool. I will commit the changes as is and will wait on guidance on how to proceed further. On Wed, Feb 6, 2013 at 10:22 AM, Andrea Aime wrote: > On Wed, Feb 6, 2013 at 5:15 PM, Justin Deoliveira wrote: > >> Just a ping on this one, wondering how to move forward. >> > > Justin, > unfortunately

Re: [Geoserver-devel] review of csw pull request

2013-02-06 Thread Andrea Aime
On Wed, Feb 6, 2013 at 5:15 PM, Justin Deoliveira wrote: > Just a ping on this one, wondering how to move forward. > Justin, unfortunately I did not find time to look into this further. The idea that there are two separate ways to translate between the internal data model and the exposed data mod

Re: [Geoserver-devel] review of csw pull request

2013-02-06 Thread Justin Deoliveira
Just a ping on this one, wondering how to move forward. On Mon, Jan 28, 2013 at 5:28 PM, Justin Deoliveira wrote: > Ok, i have updated the javadoc and pushed the discussed changes to the csw > branch in my repo. > > @Andrea: I'll wait to hear from you about when to merge in or if you think > we

Re: [Geoserver-devel] review of csw pull request

2013-01-28 Thread Justin Deoliveira
Ok, i have updated the javadoc and pushed the discussed changes to the csw branch in my repo. @Andrea: I'll wait to hear from you about when to merge in or if you think we need a bit more deeper review. On Mon, Jan 28, 2013 at 3:44 AM, Niels Charlier wrote: > Hi Justin, > > In response to your

Re: [Geoserver-devel] review of csw pull request

2013-01-28 Thread Niels Charlier
Hi Justin, In response to your questions. On 01/25/2013 05:24 PM, Justin Deoliveira wrote: > > > 1. This method wold never be called by "client" code, it is more an > internal method used to the mapping? I think it is right that it's probably not really useful to the "client" code, however it

Re: [Geoserver-devel] review of csw pull request

2013-01-25 Thread Justin Deoliveira
On Thu, Jan 24, 2013 at 6:56 AM, Niels Charlier wrote: > > >> So if I understand this correctly this means two additional changes. >> >> 1. Remove the ElementSet argument from CatalogStore.getRecords() method. >> 2. Remove CatalogStore.**translateProperty() >> >> (1) is trivial, more or less all

Re: [Geoserver-devel] review of csw pull request

2013-01-24 Thread Niels Charlier
> > So if I understand this correctly this means two additional changes. > > 1. Remove the ElementSet argument from CatalogStore.getRecords() method. > 2. Remove CatalogStore.translateProperty() > > (1) is trivial, more or less all calls to it passed in null. Made that > change and tests are happ

Re: [Geoserver-devel] review of csw pull request

2013-01-24 Thread Justin Deoliveira
On Wed, Jan 23, 2013 at 11:10 AM, Niels Charlier wrote: > Hello Andrea, > > Sorry for my late reponses. > > > On 01/19/2013 06:54 PM, Andrea Aime wrote: > > Here is some more I've found during the review. > > There's a change in DefaultCatalogFacade.java that I'm not sure about: > > if (null

Re: [Geoserver-devel] review of csw pull request

2013-01-23 Thread Niels Charlier
Hello Andrea, Sorry for my late reponses. On 01/19/2013 06:54 PM, Andrea Aime wrote: Here is some more I've found during the review. There's a change in DefaultCatalogFacade.java that I'm not sure about: if (null != sortByList) { for (int i = sortByList.length - 1; i >=0 ; i--) {

Re: [Geoserver-devel] review of csw pull request

2013-01-21 Thread Justin Deoliveira
On Sat, Jan 19, 2013 at 10:54 AM, Andrea Aime wrote: > On Thu, Jan 17, 2013 at 2:10 AM, Justin Deoliveira > wrote: > >> Hi all, >> >> I spent a few hours today reviewing the pending pull request from Niels >> that adds support for iso application profile and a catalog store based >> directly from

Re: [Geoserver-devel] review of csw pull request

2013-01-19 Thread Andrea Aime
On Thu, Jan 17, 2013 at 2:10 AM, Justin Deoliveira wrote: > Hi all, > > I spent a few hours today reviewing the pending pull request from Niels > that adds support for iso application profile and a catalog store based > directly from the geoserver catalog to the existing csw community module. > >