scm-manager / website

Home of SCM-Manager
https://scm-manager.org
GNU Affero General Public License v3.0
2 stars 4 forks source link

Migrated scm-htpasswd-plugin for scm-manager 2.x #81

Closed ggrandes closed 2 years ago

ggrandes commented 2 years ago

Proposed changes

Migrated scm-passwd-plugin from scm-manager 1.x to 2.x. Based on scm-ldap-plugin. This will fix issue 1961.

Plugin repository: https://github.com/ggrandes/scm-htpasswd-plugin

Your checklist for this pull request

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

Contributor:

Reviewer:

Checklist for branch merge request (not required for forks)

eheimbuch commented 2 years ago

Hello Guillermo,

thank you very much for your contribution. We would like to review your plugin and include it in our organization. After that we would publish it in the official plugin center. You will be the maintainer and you can request changes with pull request. If we can help you in any way, don't hesitate to ask.

Regards, Eduard

eheimbuch commented 2 years ago

Hey,

we reviewed your plugin and it works fine. There are only two things we are wondering about.

  1. Could you use your own package name? It would clarify that this code was written by someone outside the SCM Team.
  2. Would you provide documentation for this plugin? This is optional, but it could surely help out the community to understand the usage.
ggrandes commented 2 years ago

Yes. What you think about...

  1. I could use org.javastack.scm.auth.htpasswd instead of sonia.scm.auth.htpasswd. Suitable?
  2. I could add not much literature, but something functional, and move some info from README.md to docs/.., ok?
ggrandes commented 2 years ago

(1) Package renamed. (2) Docs, tomorrow.

ggrandes commented 2 years ago

(2) Docs, done

eheimbuch commented 2 years ago

Thank you very much. I tested it again and got a NPE accessing the ui for the htpasswd config page. Since it was some minor issue i fixed it myself. We will release your plugin soon!

https://github.com/scm-manager/scm-htpasswd-plugin/commit/0a63cced926f340f539530458ade041f3d575003

ggrandes commented 2 years ago

Thanks @eheimbuch 👍