Sigh... indeed the patch was there. My apologies. When I looked at the issue
I thought that the 3 patches were all the same patch just different versions
of it. So I only looked at the top one with was only geoserver app schema
changes. Again sorry for that.
About the changes in question, yes I did indeed look them over and was in
favor, but only on the trunk. Although it seems that was a mistake and we
are in agreement there.
Anyways, apologies to all for making such a big fluff. I will try to be more
careful about reviewing patches in the future.
-Justin
On Tue, Oct 19, 2010 at 8:11 PM, Niels <[email protected]> wrote:
> Hi,
>
> Hmmm, I don't understand why you would think that the changes were only to
> app-schema, the diff file has been attached to the issue since 19 sept and
> was updated on the 28th of september, including all the changes, just as you
> say it should be. Also, these changes have been discussed extensively on the
> jira issue and on this mailing list. From these discussions and your
> response on the jira issue, everyone here really assumed that you approved
> the changes. I don't really understand why you can't see the diff file, is
> that possible?
>
> The update on the branch though is indeed a mistake from our end. I guess
> this should be rolled back again.
> It was indeed agreed the changes would only be applied to trunk, so I take
> responsibility for that. My excuse is that this was my first commit and I
> was given explicit instructions to also update the branch, though my
> colleagues didn't realise that these specific update wasn't supposed to go
> to the branch.
>
> Some communication errors, I guess!
>
> Niels
>
>
>
> On 20/10/10 08:16, Justin Deoliveira wrote:
>
>
>
> On Tue, Oct 19, 2010 at 6:07 PM, Jody Garnett <[email protected]>wrote:
>
>> I am not up to speed on the discussion; although I have been trying to
>> catch up on Jira.
>>
>> I did expect some changes to XPathPropertyAccessor - but only on trunk
>> as you indicated (this was the subject of the performance discussion prior
>> to foss4g). Checking ... GEOT-3066 is correct. If you look at the first
>> comment for that Jira it does indicate that the extent of the changes ...
>> including XPathPropertyAccessorFactory.
>>
>
> Ok, so perhaps I should have read the comments a little closer. But why
> was no diff posted or updated on the issue? Usually when a jira is sent to
> somebody for review a relevant/updated patch is attached to the issue, and
> the reviewer is not expected to go through all the comments trying to track
> down a diff?
>
> And about the change at hand. Yes it was discussed in detail on the
> mailing list. I brought it up because of my reservations about how it
> affected backwards compatabilty. And there it was decided on a approach that
> would keep things backward compatible. It was also mentioned that this
> should not be done on the stable branch, but perhaps that part should have
> been more clear. Quoting from email:
>
> "This breaks backward compatibility and i think requires more
> discussion. There is much more use of geotools beyond test cases and even
> beyond geoserver and udig. If there is one thing i learned at foss4g it is
> that there are lots of people using geotools, even though they don't take
> parts in discussions on the developer list.
>
> So i think this change requires more discussion and probably a proposal.
> And it certainly can't take place on stable branch imo."
>
>
>> Jody
>>
>> On 20/10/2010, at 9:58 AM, Justin Deoliveira wrote:
>>
>> I was reviewing the commit logs today and shocked to see that recent
>> changes revolving around property access and the feature model are
>> being committed to the stable branch?
>>
>> I am also quite confused by the commit logs. For instance, I was
>> recently asked to review a patch on
>> http://jira.codehaus.org/browse/GEOT-3066. I looked it over and since it
>> only contained files in the app-schema modules i did not really have much to
>> say. But then looking at the commit that occurred:
>>
>> Author: NielsCharlier
>> Date: 2010-10-19 00:01:43 -0700 (Tue, 19 Oct 2010)
>> New Revision: 36280
>>
>> Modified:
>>
>> branches/2.6.x/modules/extension/xsd/xsd-core/src/main/java/org/geotools/xml/XPathPropertyAccessorFactory.java
>>
>> branches/2.6.x/modules/extension/xsd/xsd-core/src/test/java/org/geotools/xml/XPathPropertyAcessorTest.java
>>
>> branches/2.6.x/modules/library/main/src/main/java/org/geotools/filter/AttributeExpressionImpl.java
>>
>> branches/2.6.x/modules/library/main/src/main/java/org/geotools/filter/FidFilterImpl.java
>>
>> branches/2.6.x/modules/library/main/src/main/java/org/geotools/filter/expression/PropertyAccessors.java
>>
>> branches/2.6.x/modules/unsupported/app-schema/app-schema/src/main/java/org/geotools/data/complex/config/AppSchemaDataAccessConfigurator.java
>>
>> branches/2.6.x/modules/unsupported/app-schema/app-schema/src/main/java/org/geotools/filter/expression/FeaturePropertyAccessorFactory.java
>>
>> branches/2.6.x/modules/unsupported/app-schema/app-schema/src/test/java/org/geotools/filter/expression/FeaturePropertyAccessorTest.java
>>
>> branches/2.6.x/modules/unsupported/app-schema/app-schema/src/test/resources/test-data/TimeSeriesTest_properties.xml
>> Log:
>> provides error reporting for invalid columns in app-schema mapping file,
>> see GEOT-3066
>>
>> Ummmmm.... am I missing something here?
>>
>> Aside from the commits being way off from the jira issue I thought it
>> was discussed before in both jira and on the developer list that this change
>> was only going to be made on the trunk/unstable branch and not on 2.0.x
>> which is our current stable branch.
>>
>> Is there something I am missing?
>>
>> --
>> Justin Deoliveira
>> OpenGeo - http://opengeo.org
>> Enterprise support for open source geospatial.
>>
>>
>> ------------------------------------------------------------------------------
>> Download new Adobe(R) Flash(R) Builder(TM) 4
>> The new Adobe(R) Flex(R) 4 and Flash(R) Builder(TM) 4 (formerly
>> Flex(R) Builder(TM)) enable the development of rich applications that run
>> across multiple browsers and platforms. Download your free trials today!
>>
>> http://p.sf.net/sfu/adobe-dev2dev_______________________________________________
>> Geotools-devel mailing list
>> [email protected]
>> https://lists.sourceforge.net/lists/listinfo/geotools-devel
>>
>>
>>
>
>
> --
> Justin Deoliveira
> OpenGeo - http://opengeo.org
> Enterprise support for open source geospatial.
>
>
>
> --
> *Niels Charlier*
>
> Software Engineer
> CSIRO Earth Science and Resource Engineering
> Phone: +61 8 6436 8914
>
> Australian Resources Research Centre
> 26 Dick Perry Avenue, Kensington WA 6151
>
--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.
------------------------------------------------------------------------------
Download new Adobe(R) Flash(R) Builder(TM) 4
The new Adobe(R) Flex(R) 4 and Flash(R) Builder(TM) 4 (formerly
Flex(R) Builder(TM)) enable the development of rich applications that run
across multiple browsers and platforms. Download your free trials today!
http://p.sf.net/sfu/adobe-dev2dev
_______________________________________________
Geotools-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geotools-devel
