[Geoserver-devel] Full commit rights for Arne Kepp and David

So in asking for commit rights for Tim I realized that there are two more new TOPP employees who should get official commit rights.

Arne and David have been working in community modules, doing a rest interface for user permissions and one for configuration. See http://svn.codehaus.org/geoserver/trunk/geoserver/community/QueryUsers/
and
http://svn.codehaus.org/geoserver/trunk/geoserver/community/RESTConfig/

You can look at the author tags to see who did what. They will mostly be doing bug fixes and the like for now, and eventually will likely bring the community modules in to the main distribution, at which time we can do a full review of the code there.

We need three plus ones from current committers for each of them.

Chris

I have reviewed patches from both David and Arne and remain impressed
with the quality. +1 from me.

-Justin

Chris Holmes wrote:

So in asking for commit rights for Tim I realized that there are two
more new TOPP employees who should get official commit rights.

Arne and David have been working in community modules, doing a rest
interface for user permissions and one for configuration. See
http://svn.codehaus.org/geoserver/trunk/geoserver/community/QueryUsers/
and
http://svn.codehaus.org/geoserver/trunk/geoserver/community/RESTConfig/

You can look at the author tags to see who did what. They will mostly
be doing bug fixes and the like for now, and eventually will likely
bring the community modules in to the main distribution, at which time
we can do a full review of the code there.

We need three plus ones from current committers for each of them.

Chris

!DSPAM:4007,47335e2e236354901796417!

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

!DSPAM:4007,47335e2e236354901796417!

------------------------------------------------------------------------

_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

!DSPAM:4007,47335e2e236354901796417!

--
Justin Deoliveira
The Open Planning Project
http://topp.openplans.org

+1 here.
went thru the code, nice stuff.
May I suggest to consistently use formattings (there are mixed tabs and space
indentation) and to update class and file headers?

best regards,

Gabriel

On Friday 09 November 2007 11:27:50 pm Justin Deoliveira wrote:

I have reviewed patches from both David and Arne and remain impressed
with the quality. +1 from me.

-Justin

Chris Holmes wrote:
> So in asking for commit rights for Tim I realized that there are two
> more new TOPP employees who should get official commit rights.
>
> Arne and David have been working in community modules, doing a rest
> interface for user permissions and one for configuration. See
> http://svn.codehaus.org/geoserver/trunk/geoserver/community/QueryUsers/
> and
> http://svn.codehaus.org/geoserver/trunk/geoserver/community/RESTConfig/
>
> You can look at the author tags to see who did what. They will mostly
> be doing bug fixes and the like for now, and eventually will likely
> bring the community modules in to the main distribution, at which time
> we can do a full review of the code there.
>
> We need three plus ones from current committers for each of them.
>
> Chris
>
>
>
> !DSPAM:4007,47335e2e236354901796417!
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Splunk Inc.
> Still grepping through log files to find problems? Stop.
> Now Search log events and configuration files using AJAX and a browser.
> Download your FREE copy of Splunk now >> http://get.splunk.com/
>
> !DSPAM:4007,47335e2e236354901796417!
>
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> Geoserver-devel mailing list
> Geoserver-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/geoserver-devel
>
>
> !DSPAM:4007,47335e2e236354901796417!

Chris Holmes ha scritto:

So in asking for commit rights for Tim I realized that there are two more new TOPP employees who should get official commit rights.

Arne and David have been working in community modules, doing a rest interface for user permissions and one for configuration. See http://svn.codehaus.org/geoserver/trunk/geoserver/community/QueryUsers/
and
http://svn.codehaus.org/geoserver/trunk/geoserver/community/RESTConfig/

You can look at the author tags to see who did what. They will mostly be doing bug fixes and the like for now, and eventually will likely bring the community modules in to the main distribution, at which time we can do a full review of the code there.

We need three plus ones from current committers for each of them.

I'm +1 for bug fixes having seen some patches from both of them.

For the new modules, they do look fine, yet I'd suggest a bit more care on the code. So far I've seen only the QueryUser module, a quick
review suggests the following:
* the files have no header with a license (so one would assume they
   are public domain, no?). The same goes for the EditableUserDAO class
   that has been committed into main (committing into "main" is fine,
   not having the license header is not)
* same as Gabriel, it seems the code does not respect the GeoServer
   coding conventions and uses a mix of tabs and spaces as well
* there is some code out of the module's purpose such as DummyRestlet.
   It's ok for the time being, would be nice to remove it before the
   module graduates to an official one.
* UserResource and UserReslet seem to be redundant (that is, doing
   the same work?). Same goes for QueryUsers.
* there is no way to list all users? Having a UsersResoure would
   be nice.
* In UserResource the plain text read and write formats seem to be
   different (they should be equal).
   The case where a different mime type is requested or provided
   is not handled explicitly.

Instructions are missing to actually try out the service. From
what I could gather:
* in main, it's necessary to replace the user dao registration
   in applicationContext.xml from GeoserverUserDao to EditableUserDAO
   (or there will be a class cast exception trying to access users
    in the REST interface)
* in web.xml it's necessary to register the path to /users as
   something handled by the main dispatcher

Even after doing that, when I try to access a user I get no
output at all (but no exception either?)

Well, that's what I could gather in a quick review. The code
looks promising, but needs some more love :slight_smile:

Cheers
Andrea