search

deepy / sonar-crowd

GNU Lesser General Public License v3.0
32 stars 37 forks source link

Enhance crowd-plugin to same basic featureset as the LDAP plugin #1

Closed ferdinandhuebner closed 10 years ago

ferdinandhuebner commented 10 years ago

This pull request enhances the existing crowd-plugin to the same basic featureset and behaviour as the LDAP plugin:

jblievremont commented 10 years ago

As you might have noticed, there was few activity around this plugin in the past months. Would you feel comfortable about obtaining push rights on the repository, eventually taking ownership of the crowd plugin?

jblievremont commented 10 years ago

Hi @ferdinandhuebner, sorry to insist but we currently don't have the necessary infrastructure here at SonarSource to test the changes you have made.

If you feel comfortable with the idea of becoming maintainer of the Crowd plugin, we will do the required changes to give you push access to the repository and rights on the CodeHaus JIRA for issue management if needed.

If you prefer to stick to a contributor role, or without an answer from you part in the next week, it's fine with us: we will turn to the mailing lists to see if someone else is interested in maintaining this plugin, then eventually proceed with the release.

Thanks!

ferdinandhuebner commented 10 years ago

I apologize for the delay, I was rather busy during the last weeks.

I'm not sure if I can take over the plugin just yet. My knowledge of the sonar codebase and the way SonarSource does things is very limited. The crowd plugin was easy to extend as I basically just copied the structure and behaviour of the LDAP plugin. It would make sense to me if somebody from either SonarSource or the sonar community would help me in some way. That should include a review of the code I wrote and some kind of quick introduction on how the sonar community releases plugins.

I didn't look into backwards compatibility with crowd versions yet and hope that I can do that within the next couple of days.

CSchulz commented 10 years ago

I can try to help you in some way, I have made some changes in my own repository but I didn't know how to merge it: https://github.com/CSchulz/sonar-crowd

aheritier commented 10 years ago

Whoau @ferdinandhuebner this is a great PR improving a lot the plugin. I'm reviewing it and will give you more feedbacks soon. It is working fine but I'll document changes to apply in the plugin configuration. Perhaps we'll have to release it as 2.0

aheritier commented 10 years ago

I finished my review @ferdinandhuebner. It is a really good contribution. I tested it on my environment and it's working like a charm (Sonar 4.4 + crowd 2.7.1). You're fixing with your PR almost all issues opened about this plugin: https://jira.codehaus.org/browse/SONARPLUGINS-1437 https://jira.codehaus.org/browse/SONARPLUGINS-3348 https://jira.codehaus.org/browse/SONARPLUGINS-2931 https://jira.codehaus.org/browse/SONARPLUGINS-3028 https://jira.codehaus.org/browse/SONARPLUGINS-1046 https://jira.codehaus.org/browse/SONARPLUGINS-3083 I added few changes on my side https://github.com/aheritier/sonar-crowd I think we can merge all of that in the official codebase, update docs and release it.

jblievremont commented 10 years ago

@aheritier thanks a lot for the review!

Do you wish to take ownership of the plugin? We have been looking for someone to take the lead on this for the past 2 months, but it seems that nobody was interested, and I am sure that this new release to come will be beneficial to many users of Crowd.

aheritier commented 10 years ago

Let's go !!! Already in production on https://sonar.exoplatform.org :-)

On Thu, Aug 7, 2014 at 3:21 PM, Jean-Baptiste Lièvremont < notifications@github.com> wrote:

@aheritier https://github.com/aheritier thanks a lot for the review!

Do you wish to take ownership of the plugin? We have been looking for someone to take the lead on this for the past 2 months, but it seems that nobody was interested, and I am sure that this new release to come will be beneficial to many users of Crowd.

— Reply to this email directly or view it on GitHub https://github.com/SonarCommunity/sonar-crowd/pull/1#issuecomment-51470092 .

Arnaud Héritier http://aheritier.net Mail/GTalk: aheritier AT gmail DOT com Twitter/Skype : aheritier

CSchulz commented 10 years ago

I think it is a good choice :) I can't do it anymore, I have left the company and there is no environment for me to develop and test. :) Good luck!

ferdinandhuebner commented 10 years ago

Thanks for the review @aheritier We're currently using the plugin with crowd 2.7.2 in a production environment. It would be great if you were to take ownership of the plugin. I can help with fixing bugs, we're interested in keeping it working with the current featureset.

You were able to get rid of the exceptions? I never encountered those.

The plugin is compatible with crowd 2.1.0 or later. I tested it by going a backwards from crowd 2.7 until I found a version that didn't work any more...

The documentation in README.md should provide a starting point for updating the plugin documentation on docs.codehaus.org. Please note that I was unable to implement a fallback authentication if the crowd server is unavailable.

aheritier commented 10 years ago

Thanks for the review @aheritier We're currently using the plugin with crowd 2.7.2 in a production environment. It would be great if you were to take ownership of the plugin. I can help with fixing bugs, we're interested in keeping it working with the current feature set.

Thanks a lot. Yes I will and take the ownership. I'm using for a very long time sonar and crowd together thus it's time to help the team if they require it.

You were able to get rid of the exceptions? I never encountered those.

Yes, That's why I updated my comment. I found the error 2 minutes after posting it. In fact the sonar.url config value has changed with the usage of the REST client. Instead of being http(s)://server[:port]/[sonarContext/]service like for the plugin in version 1.0 it must be http(s)://server[:port]/[sonarContext/]. For that I will propose to publish your work under a version 2.0 with a big warning explaining to read the wiki before upgrading. I'll add a 1.0->2.0 migration part.

The plugin is compatible with crowd 2.1.0 or later. I tested it by going a backwards from crowd 2.7 until I found a version that didn't work any more...

cool. For sonar, I suppose it will work with 3.0+ but I didn't verified myself. I don't know if at SonarSource they have some tools to validate a plugin with several Sonar versions.

The documentation in README.md should provide a starting point for updating the plugin documentation on docs.codehaus.org.

yes, it helped me a lot when I started to review your work

Please note that I was unable to implement a fallback authentication if the crowd server is unavailable.

ok. We'll create an issue for it to be sure to not forget.