I rolled back on the branches...
On 20/10/10 10:43, Justin Deoliveira wrote:
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] <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
--
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
