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]
<mailto:[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 <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]
<mailto:[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
------------------------------------------------------------------------------
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
