Hey Jody OK, so I decided to send everything as a single patch. Other comments below..
>> 1. Question: why not put all setters in the org.geotools.styling.Symbolizer >> interface? >> . Example: setGeometryPropertyName() >> . Wouldn't it be nice to have an abstract SymbolizerImpl? > > That will be fine. Also note andrea/myself have an idea of allowing > some geometry processing here but are stuck on some details. OK, I put the setUnitOfMeasure() as a method in the Symbolizer interface, but didn't create the AbstractSymbolizerImpl >> 4. PointSymbolizerImpl initializes UOM to NonSI.PIXEL by default. I think > > I would much prefer that; I hate null in an API. OK, but I think I wasn't clear: the patch I sent uses null for PixelSymbolizerImpl since that was the behavior for everything else. > >> 5. The original idea was to create an SEParser (effectively an SLD+SEParser) >> that would contain the changes for UOM, etc. Do you still want to do it that >> way, Jody? In any case, the same would have to be done for SLDTransformer to >> enable UOMs when serializing (i.e., create an SETransformer as well) As I said on the patch, I ended up sending changes to SLDParser/Transformer to ease the reviewing > >> 6. Problems in DuplicatingStyleVisitor: > > I have another implementation I can commit if you like. Do you think you could take a look at the version I sent in the patch? You could decide then what to commit.. >> 9. Problem with FilterTransformer > > I tried removing this and could not pass test cases; it would be good > to try for a rematch. Well, I changed that test case for that one, since it really looked wrong. It's weird too that there was ONE single test there, just to test the IdFilter. Looks like the person who put the <Filter></Filter> tags was the same one who did the test.. > >> 10. Problem with GraphicImpl.setExternalGraphics() and setMarks() >> . Was clearing up all graphics, regardless of class (e.g., >> setExternalGraphics() was clearing the Marks and vice-versa) > > I find these methods to be really not very clear; I changed the > StyleFactoryImpl to use the graphics list directly; and would like to > see these setters retired. I actually imagine they should go through > the graphics list and remove all the Marks (and then set the marks > provided). That's exactly what I did. >> A final note: I tried updating GeoTools and running Maven here today and it >> complained that it couldn't find jar-collector-2.6-SNAPSHOT.jar. It seems to >> be looking in the OSgeo repository for plugins, which I guess is wrong (it >> should be Opengeo now right?) > > I think we are trying to cut down on the dependencies these days; I > just did a fresh checkout on a new machine (using maven 2.1.0) and was > able to build (after fixing one small pom.xml in an unsupported > module). As you must have seen, I think I fixed the pom.xml for the <pluginsRepository> tag (was pointing at OSgeo for snapshots, not OpenGeo). Worked here. > > Thanks again Milton :-) And I thank you for the comments and support! I've been slow to report things back, but I hope it's useful anyways :) Milton -- Milton Jonathan Grupo GIS e Meio Ambiente Tecgraf/PUC-Rio Tel: +55-21-3527-2502 ------------------------------------------------------------------------------ OpenSolaris 2009.06 is a cutting edge operating system for enterprises looking to deploy the next generation of Solaris that includes the latest innovations from Sun and the OpenSource community. Download a copy and enjoy capabilities such as Networking, Storage and Virtualization. Go to: http://p.sf.net/sfu/opensolaris-get _______________________________________________ Geotools-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/geotools-devel
