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
--
Justin Deoliveira
The Open Planning Project
jdeolive@anonymised.com
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
Hi Lucas,
I added some xstream persistence classes which will automagically persist and unpersist objects. In my local checkout i hooked it up for WPS andn it works :).
Let me know if you want me to commit it. Basically all it amounts to is creating a subclass of XStreamServiceLoader and registering it in a spring context.
-Justin
Lucas Reed wrote:
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
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
!DSPAM:4007,483f23a233922458217002!
--
Justin Deoliveira
The Open Planning Project
jdeolive@anonymised.com
Hi Justin,
OK cool, yeah please commit it.
-Lucas
Justin Deoliveira wrote:
Hi Lucas,
I added some xstream persistence classes which will automagically persist and unpersist objects. In my local checkout i hooked it up for WPS andn it works :).
Let me know if you want me to commit it. Basically all it amounts to is creating a subclass of XStreamServiceLoader and registering it in a spring context.
-Justin
Lucas Reed wrote:
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
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
!DSPAM:4007,483f23a233922458217002!