contrib
contrib copied to clipboard
Contrib Group Application: shashankthigale11 + oauth_client
Hello and welcome to the contrib application process! We're happy to have you :)
Please indicate how you intend to help the Backdrop community by joining this group
- Option 1: I would like to contribute a project
- Option 2: I would like to maintain a project, but have nothing to contribute at this time
- Option 3: I would like to update documentation and/or triage issue queues
Based on your selection above, please provide the following information:
Option 1: I would like to contribute a project.
Name of the module: OAuth Client
This module is not ported from Drupal 7 and its a completely new module.
(option 1) Please note these 3 requirements for new contrib projects:
- [x] Include a README.md file containing license and maintainer information. You can use this example: https://raw.githubusercontent.com/backdrop-ops/contrib/master/examples/README.md
- [x] Include a LICENSE.txt file. You can use this example: https://raw.githubusercontent.com/backdrop-ops/contrib/master/examples/LICENSE.txt.
- N/A: ~~If porting a Drupal 7 project, Maintain the Git history from Drupal~~.
Post a link to your new Backdrop project under your own GitHub account (option 1) https://github.com/shashankthigale11/oauth_client
If you have chosen option 2 or 1 above, do you agree to the Backdrop Contributed Project Agreement YES
Hi @shashankthigale11, I'm sorry for the delay. I'll try to get some eyes on this application.
Hey @shashankthigale11 👋🏼 ...apologies for the delay here, but the community has been busy with the upcoming minor core release. I'm having a look at your application now 👀
This module is not ported from Drupal 7 and its a completely new module.
Confirming that the namespace https://www.drupal.org/project/oauth_client does not exist 👍🏼 (only https://www.drupal.org/project/oauth2_client and https://www.drupal.org/project/openid_connect which has "OAuth client" in the project human-readable name, but no namespace clash). No clash in Backdrop contrib either 👍🏼
@shashankthigale11 can you please update this line in the README?:
Ported to Backdrop CMS by Shashank Thigale.
Since the module has not been ported over from Drupal, that line should be better as "Created for Backdrop by ...".
Both a README.md and an appropriate LICENSE.txt are included in the project, so we're all good there 👍🏼
Before we can make a final review and approve your application, I was wondering if you can please take the time to address the following things:
-
Go through the documentation available here, and fix the formatting in your code accordingly: https://docs.backdropcms.org/documentation/coding-and-documentation-standards ...I specifically (only quickly) checked
oauth_client_config.incandoauth_client.moduleand I see quite a few issues that should be addressed. That would make the initial review here easier, but also allow easier contributions by others in the community in the future. -
I noticed that there is no configuration currently in
oauth_client.settings.json. Is that intentional? -
There is a
test_mo_config()function and atest_mo_config()function in the .module file. Are these needed there? If so, then they should be prefixed withoauth_client_I believe. -
Then in
oauth_client_uninstall()I see that you are doing a$config->clear(). I believe that that shouldn't be required if you implementhook_config_info(). Can you please try that instead? -
I also tried installing the module in a demo instance via https://backdropcms.org/demo, but got the following error when I tried to enable the module:
Failed opening required '/var/lib/tugboat/backdrop/modules/oauth_client/includes/Handler.php' (include_path='.:/usr/local/lib/php')
I believe that that is because of the way that the required libraries are being loaded in the .module file:
require_once BACKDROP_ROOT . '/modules/oauth_client/includes/Handler.php'; require_once BACKDROP_ROOT . '/modules/oauth_client/includes/Utilities.php';You can see other examples in the rest of the contrib modules for Backdrop for clues on how to do that. I quickly found this for you (from the popular devel module): https://github.com/backdrop-contrib/devel/blob/1.x-1.x/devel.module#L426C3-L426C107
@include_once BACKDROP_ROOT . '/' . backdrop_get_path('module', 'devel') . '/lib/krumo/class.krumo.php';...but I also found a suggestion in https://stackoverflow.com/questions/31946670/how-to-include-a-php-class-in-drupal-7 that says that something like this should also work (note: untested!):
module_load_include('php', 'mymodule', '../myclass.class');Since I wasn't able to install the module, I couldn't review anything further.
Apologies for the lengthy list of items I am requesting above - I hope that it doesn't deter you from contributing. Let me know when you have addressed the above issues, and I will have another look then ...or if you have any questions or need help with any of the above items in the meantime, then also let us know and we can assist you.
Thank you for contributing to Backdrop! 🙏🏼
@shashankthigale11 I also wanted to say that you can consider joining us in our official Zulip chat to get answers to any questions faster (you can use your GitHub account to join). You are free to join any existing thread, or start a new one, but this is where we are handling notifications about contrib applications: https://backdrop.zulipchat.com/#narrow/stream/218635-Backdrop/topic/Contrib.20Application
If you have any question, or you think we are taking too long to reply here, then please drop a message there, and our friendly community members will be happy to assist you :)
I recommend using a namespace that makes it more explicit that this is for only Azure and related. Currently oauth_client makes it seem like it's much more generic.
@shashankthigale11 It looks like you've gotten a lot of feedback, but all of that is intended to be helpful - none of it is a requirement to join Backdrop Contrib. Since it appears that you have met the only 2 actual requirements...
- [x] Include a README.md file containing license and maintainer information. You can use this example: https://raw.githubusercontent.com/backdrop-ops/contrib/master/examples/README.md
- [x] Include a LICENSE.txt file. You can use this example: https://raw.githubusercontent.com/backdrop-ops/contrib/master/examples/LICENSE.txt.
- N/A: ~~If porting a Drupal 7 project, Maintain the Git history from Drupal~~.
An invitation to join the Backdrop Contrib group is on the way! 🎉 Please check your inbox and accept the invitation :)
Once you have accepted the invitation, feel free to transfer the repository into the backdrop-contrib group at any time (ask here if you have questions).
@shashankthigale11 we are going to close this issue as complete, but if you have any questions or need any assistance feel free to open a new issue.