Thanks for the review Andrea.

>>
>> I have found xstream to be very flexible and straight forward. 
>> Tweaking the default settings and adding in some custom stuff I have 
>> been able to output xml which looks pretty close to what we actually 
>> output today.
>>
>> Comments/concerns/feedback welcome.
> 
> The code looks nice and clean, the effort to extend it seems low,
> it seems possible to achieve extension by adding extensions points
> so that other plugins do configure the XStream instance for their
> needs. I like it.
> 
Yeah I thought about extensibility. We could have the class process an 
extension point. However, my thought was that if some code needs to 
customize the xstream in some way (adding a converter or omitting a 
field or something) they can just grab the reference to the XStream 
object and modify it.

> Questions:
> * how is the final layout of the xml files like?
With all the converters in that class similar to what we have today. 
Example:

<catalog>
   <stores>
     <dataStore>
       <id>foo</id>
       <name>foo</name>
       <enabled>false</enabled>
       <workspace>foo</workspace>
       <connectionParameters/>
       <metadata/>
     </dataStore>
   </stores>
   <namespaces>
     <namespace default="true">
       <id>acme</id>
       <prefix>acme</prefix>
       <uri>http://acme.org</uri>
     </namespace>
   </namespaces>
   <workspaces>
     <workspace default="true">
       <id>foo</id>
       <name>foo</name>
     </workspace>
   </workspaces>
   <layerGroups/>
   <styles>
     <style>
       <id>style</id>
       <name>style</name>
       <filename>style.sld</filename>
     </style>
   </styles>
</catalog>

We could even do more to make it look the same.

> * is this limited to save just the services for the moment, and in
>   particular only wfs?
> * what about the catalog?
Yeah at the moment only wfs is hooked up to xstream stuff. But hooking 
up the otehr stuff is pretty easy. Same goes for catalog and feature 
type / coverage infos. Not hooked up but not too much work.
> * why are you not persisting the client properties out of the GeoServer
>   object? (xs.omitField(GeoServerInfoImpl.class, "clientProperties");)
The idea here was that "metadata" would be persisted and 
"clientProperties" would be transient. We could just remove 
"clientProperties" all together. I cant think of a good use case for 
them at the moment.
> 
> Cheers
> Andrea
> 
> 
> !DSPAM:4007,486374503595219720167!
> 


-- 
Justin Deoliveira
The Open Planning Project
[EMAIL PROTECTED]

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Geoserver-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

Reply via email to