On Wed, Aug 29, 2012 at 4:19 PM, Andrea Aime
<andrea.a...@geo-solutions.it>wrote:

> On Wed, Aug 29, 2012 at 3:45 PM, David Winslow <dwins...@opengeo.org>wrote:
>
>>
>> https://github.com/Adam-Brown/geotools/compare/geotools:master...Adam-Brown:master
>
>
> Heck, I did try that (following a github blog) and kept on getting a 404..
> I guess I must have mis-typed
> something.
>
> Thanks for the working link :)
>

Hum... looking at the link David provided it seems there are files that
have been reformatted top
to bottom, this makes it impossible to see where the changes are
(AppSchemaResolver for example, but also ContentFeatureCollection,
SimpleFeatureTest).

What is the meaning of ContentComplexFeatureSource? It's basically empty,
and if someone forgets
to override all of the methods they will get a broken implementation, there
are no optional methods
in FeatureSource (that I remember of). Same
for WFSContentComplexFeatureCollection

There are some changes to ContentFeatureCollection that are not explained,
e.g.:

 public final ContentFeatureCollection getFeatures() throws IOException {
516 516
         Query query = joinQuery(Query.ALL);
517
-        return new ContentFeatureCollection( this, query );
  517
+        return getFeatures(query );
518 518
     }

If this is a fix for some bug please explain, if it's not please don't
change someone else
coding style to match yours, we're many around here if everybody starts
doing that we'll
just end up making a great mess.

AbstractDataAccessFactory has been deprecated and replaced with a base class
that generates only DataAccess, I don't like this, if forces uses to switch
to DataAccess
interfaces even if they don't want to, please remove the deprecation.

DataAccessFinder seems to have a mix of reformats and changes, again it's
close to
impossible to say what really changed.

H2DataStoreFactoryTest has been changed without discussion and it's
unrelated to the
proposal, this should be discussed as a separate issue instead. As usual
the class seems
to suffer from reformatting, same goes for H2TestSetup.

PropertyDataStore2Test has also been reformatted.

The WFS part I won't comment on, but hope Gabriel will have a look at it.

Long story short, in the current state the changes are not worth reviewing
and cannot
be committed, anyone actually looking at the patch should have come to the
same
conclusion: -1

Please remove all the formatting changes and let's try again :-)

Cheers
Andrea

-- 
==
Our support, Your Success! Visit http://opensdi.geo-solutions.it for more
information.
==

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054  Massarosa (LU)
Italy
phone: +39 0584 962313
fax:   +39 0584 962313
mob:   +39  339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
GeoTools-Devel mailing list
GeoTools-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Reply via email to