Reviewing the commits more closely I definitely have a few concerns 
which affirms my preference to have these changes reviewed before hand.

* The method added to catalog builder seems unnecessary to me. I think 
just the plain buildLayer can be called, and then properties overrideen 
after the fact, instead put putting them into a map and passing it into 
the method. It also does not sync up with any of the other methods in 
that class. I would like to keep that class as consistent as possible.

* You have made changes to api without any discussion. Supporting 
wmspath and and style as query parameters is not documented as api options:

http://gridlock.openplans.org/geoserver/1.7.x/doc/user/extensions/rest/rest-config-api.html#coverage-stores

Not saying we should not have them, but this is an extension now, not a 
community module anymore, such things require discussion before hand.

* Many of the changes seem a bit arbitrary, changing variable 
declarations to final for instance. Again, for consistency sake I prefer 
that you delegate to how the rest of the class / method does things when 
making chagnes.

* Setting the layer name as the entity is again an api change, and one I 
don't agree with. It is more RESTful to set the location of the resource 
using the appropriate HTTP header, 'Location' like we do for the POST 
case. Also, the indication of where the resource should be down with a 
full uri, just returning the name assumes that URI's are non-opaque, ie 
the client has pre-knowledge of how URI's are built, which is another 
REST nono.

-Justin

Justin Deoliveira wrote:
> Hi Daniele,
> 
> I see you are making commits on 1.7.x to restconfig and main. I would 
> appreciate posting patches for pre review first before committing. 
> Especially in this case where we have not yet wrapped up discussion 
> about how to handle the PUT issue from previous email.
> 
> I am not trying to hamper work here but I have been deemed the 
> maintainer of that extension, and while there are no hard rules in place 
> about who is allowed to commit, i would appreciate being to review 
> commits before they go through.
> 
> -Justin
> 


-- 
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.

------------------------------------------------------------------------------
The NEW KODAK i700 Series Scanners deliver under ANY circumstances! Your
production scanning environment may not be a perfect world - but thanks to
Kodak, there's a perfect scanner to get the job done! With the NEW KODAK i700
Series Scanner you'll get full speed at 300 dpi even with all image 
processing features enabled. http://p.sf.net/sfu/kodak-com
_______________________________________________
Geoserver-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

Reply via email to