Hello Andrea,
thanks for your review.
I fixed the typos and renamed the WMS paramters so that are more generic now.
Also I created a new patch containing the geoserver doc tutorial also using SVN diff which should be readable to everyone.
Attached it to a new JIRA so we don’t need to move the patch around (http://jira.codehaus.org/browse/GEOS-4734)
About the ExecutorService you are right, actually one solution I implemented in this new patch is to improve a bit the WMSLifecycleHandler and moving there the ExecutorService management.
Let me know if you find some other issues or improvements, and thanks for your effort.
Cheers,
Alessio.
Ing. Alessio Fabiani
Founder / CTO GeoSolutions S.A.S.
GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: (+39) 0584 96.23.13
fax: (+39) 0584 96.23.13
mobile:(+39) 331 62.33.686
On Tue, Aug 23, 2011 at 3:11 PM, Andrea Aime <andrea.aime@anonymised.com> wrote:
On Mon, Aug 22, 2011 at 7:35 PM, Alessio Fabiani
<alessio.fabiani@anonymised.com.> wrote:
Hello guys,
atached to the email the patch for the GSIP 62 - WMS Animator.
Would it be possible for you to have a quick code review and feedbacks?
Had a quick look at the patch, briefly:
- if possible next time a svn/git patch would be better, this one
assumes the reviewer
uses Eclipse and has the same project names as you
- Animator javadoc typo: uncompleted → incomplete
- gifMaxAllowedFrames → should probably be max allowed frames,
general, we can have
avi or flash animator generators and the max frames would apply to
them as well
- the concurrent setup makes every request actually generate one frame
at a time.
Is this intended?
Besides that the patch is clean, has tests, follows the coding
conventions, looks good
Cheers
Andrea
–
Ing. Andrea Aime
GeoSolutions S.A.S.
Tech lead
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 962313
http://www.geo-solutions.it
http://geo-solutions.blogspot.com/
http://www.youtube.com/user/GeoSolutionsIT
http://www.linkedin.com/in/andreaaime
http://twitter.com/geowolf
Get a FREE DOWNLOAD! and learn more about uberSVN rich system,
user administration capabilities and model configuration. Take
the hassle out of deploying and managing Subversion and the
tools developers use with it. http://p.sf.net/sfu/wandisco-d2d-2
Geoserver-devel mailing list
Geoserver-devel@anonymised.comsts.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel