Hi Justin,

Thanks for the review.

We're going to hold off on the DTO stuff for now.

I'm currently working on getting the new configuration system hooked up.

-Lucas


Justin Deoliveira wrote:
> Hi Lucas,
> 
> I just did a quick review of the wps module you committed, and here is a 
> bit of feed back for you:
> 
> 1. DTO
> 
> The module did not compile due to the lack of a WPSDTO class. We can 
> discuss your requirements but I would forgo using the DTO method for 
> now. The only reason to have it would be if you were planning on build a 
> UI for the wps module with the current struts UI stuff.
> 
> 2. Configuration
> 
> Using the new configuration. This involves creating a WPSInfo class and 
> plugging into the new configuration with a "WPSLoader". If all this is 
> foreign to you do not worry, it was just added yesterday as part of the 
> configuration changes. This would also be an interesting chance to try 
> out persistence with xstream (see the configuration proposal for 
> details). It would be quite simple (a few lines of code) and your 
> configuration would be automagically persisted.
> 
> 3. Application Context
> 
> Minor, but you have a mean called "xmlReader-1.0.0" in your application 
> context. This clashes with a bean in the xml module and causes an 
> exception on startup. I would rename it to "wpsXmlReader-1.0.0", we 
> should also do the same for the wfs bean.
> 
> Thats it. The rest looks good. With a few minor modifications I was able 
> to successfuly execute a wps getCapabilities request. Yay!! If you are 
> interested I can commit said modifications, or I can post a patch for 
> you, or we can discuss them further. Whichever you prefer.
> 
> Great work.
> 
> -Justin
> 



-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
Geoserver-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

Reply via email to