Contrib Group Application: rbargerhuff (cas module)

[rbargerhuff]

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
Option 2

My team would like to start the process of providing initial ports of certain modules for the Backdrop community. My team would also like to contribute to Backdrop community by submitting bugs and fixes, etc. For example we have already contributed to the coder_upgrade module and made posts regarding initial port requests for a wealth of Drupal 7 modules.

Based on your selection above, please provide the following information:

(option #1) The name of your module, theme, or layout
cas, coder_upgrade, other modules my team has planned.

(option #1) Please note these 3 requirements for new contrib projects:

• 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
• Include a LICENSE.txt file.
  You can use this example: https://raw.githubusercontent.com/backdrop-ops/contrib/master/examples/LICENSE.txt.
• If porting a Drupal 7 project, Maintain the Git history from Drupal.

(option #1 -- optional) Post a link here to an issue in the drupal.org queue notifying the Drupal 7 maintainers that you are working on a Backdrop port of their project

• https://www.drupal.org/project/cas/issues/2914520
• https://www.drupal.org/project/menu_token/issues/3261166
• https://www.drupal.org/project/rules_conditional/issues/3261174
• https://www.drupal.org/project/seckit/issues/3261178
• https://www.drupal.org/project/views_php/issues/3261182

Post a link to your new Backdrop project under your own GitHub account (option #1)
https://github.com/rbargerhuff/cas

(option #2) If you have already contributed code to Backdrop core or contrib projects, please provide 1-3 links to pull requests or commits

• Question/Concern - Conversion to Backdrop Breaks Entity API Functionality backdrop-contrib/coder_upgrade#75
• Addresses typed class properties. backdrop-contrib/coder_upgrade#74 (smaiorana is part of my team)
• Addresses issue #56. backdrop-contrib/coder_upgrade#57

If you have chosen option #2 or #1 above, do you agree to the Backdrop Contributed Project Agreement
YES

Once we have a chance to review your project, we will check for the 3 requirements at the top of this issue. If those requirements are met, you will be invited to the @backdrop-contrib group. At that point you will be able to transfer the project.

OK

Please note that we may also include additional feedback in the code review, but anything else is only intended to be helpful, and is NOT a requirement for joining the contrib group.

OK

Cheers!

[jenlampton]

Hello @rbargerhuff and thanks for offering to join the contrib group!

I'm going to use the https://github.com/rbargerhuff/cas project to review the list of requirements for joining the contrib group:

• 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
• Include a LICENSE.txt file.
  You can use this example: https://raw.githubusercontent.com/backdrop-ops/contrib/master/examples/LICENSE.txt.
• If porting a Drupal 7 project, Maintain the Git history from Drupal.

Would you please add a README.md file to this project? We recommend coping the example file provided, and then merge in any relevant information from the previous README.txt file, carefully noting which sections are required for Backdrop projects.

Would you please also include a LICENSE.txt file. We recommend copying the example file provided.

I can see all the commits from the Drupal history, so thank you for taking care of that item :)

Once these requirements are met, I will be able to send an invitation off for you to join.

---

It is worth noting that even though the branch name is 1.x-1.x it does not look like this project will work on a Backdrop website yet, as the info file still indicates that this module is only compatible with Drupal 7.

I usually like to do a detailed code review when Drupal modules are ported, in the hopes that the information might help with the transition for those new to Backdrop :)

However, since it appears that no work has been done yet to make this project work for Backdrop, I'll hold off on the review until you are ready. Please add a comment to this issue if/when you are interested in that review :)

[rbargerhuff]

Hi @jenlampton thank you so much for your response. Everything you asked for was already included in the backdrop-compatibility branch. I followed this guide How to Maintain Contrib Modules for Drupal and Backdrop at the Same Time which referenced creating that branch's purpose and name.

[jenlampton]

Everything you asked for was already included in the backdrop-compatibility branch.

Ah, I'm sorry. I didn't think to look for that branch when I saw the main branch was named 1.x-1.x. Thank you for pointing me in the right direction!

• 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
• Include a LICENSE.txt file.
  You can use this example: https://raw.githubusercontent.com/backdrop-ops/contrib/master/examples/LICENSE.txt.
• If porting a Drupal 7 project, Maintain the Git history from Drupal.

An invitation to join the contrib group is on the way! I'll post a code review in the next comment :)

[rbargerhuff]

Ah, I'm sorry. I didn't think to look for that branch when I saw the main branch was named 1.x-1.x. Thank you for pointing me in the right direction!

No worries!! I used that branch because I didn't want to merge into 1.x-1.x until it was tested by the community. I do apologize because I do not fully understand the process yet. My team is going on the presumption that once it has been tested it can then be officially merged into 1.x-1.x.

[jenlampton]

I'm also going to do a quick code review, but items below are only suggestions. Take or leave them as you see fit!

As stated before, we do this review specifically looking for things that may be done differently between Backdrop and Drupal. We hope that this review might help with the transition for those new to Backdrop :)

README.md suggestions

• Backdrop's minimum version was just bumped to PHP 5.6, so you are welcome to increase the PHP version requirement number to match.
• Installation section still refers to Drupal. Standard Backdrop installation instructions can be copied from the example readme file.
• small typo in maintainers section (missing [) that breaks the link.
• The link to issue queue will need to be updated when the module is moved to contrib.

.install file suggestions

• In hook_uninstall() it's not necessary to delete the module's own config files or values within. As long as a module properly implements hook_config_info() that cleanup should happen automatically when the module is uninstalled. (If, however, a module writes to config files provided by other modules, it should remove its own values from those other files)

.module file suggestions

• The hook_help implementation can be removed. (That hook does not exist in Backdrop.)

.tokens.inc suggestions

• The @file docblock still references "Token module" but token is no longer a stand-alone module in Backdrop, it has been integrated into core.

Location of files within the module

• We recommend that all .css files go within a /css/ subdirectory. (It doesn't matter so much when you only have one js file, but if there were a handful this would help keep things tidy)
• We recommend that all .js files go within a /js/ subdirectory. (It doesn't matter so much when you only have one js file, but if there were a handful this would help keep things tidy)
• We recommend that all .test files go within a /tests/ subdirectory. Test modules should also be placed within that directory, but also in a directory matching the name of the module. In this case: /tests/cas_test/.
• The tests.info file should also be moved within the /tests/ subdirectory, and the file lines in the .tests.info file should be updated to remove the tests directory - so that they are still relative. (Keeping the test code together, and out of the way, makes it easier for people to immediately see what the module provides when scanning the list of files within)

General coding standards

• Blank lines after the opening <?php tag can be removed if you want. The extra line breaks were required in Drupal because of the previous CVS version control system, but are not needed anymore.
• Inline code comments should start with // followed by a space, and end with a period .. (See line 309 & 732 of cas.module, among others)

Overall this module looks fantastic -- Nice work on the port, and thank you for contributing it!

[rbargerhuff]

Hi Jen,

Okay, we are going to review and make the changes as you suggested. I appreciate the code review and will do a final commit for these changes before moving to backdrop-contrib group.

One thing to note, the Backdrop CMS Hooks section of the documentation uses hook_help() as an example. I think that is why my team thought hook_help() was still implemented.

https://docs.backdropcms.org/api/backdrop/core!includes!module.inc/group/hooks/1

The available hooks to implement are explained here in the Hooks section of the developer documentation. The string "hook" is used as a placeholder for the module name in the hook definitions. For example, if the module file is called example.module, then hook_help() as implemented by that module would be defined as example_help().

Thank you and I am glad I could contribute to the Backdrop community. We are looking forward to a bright future!!

Cheers!!

[jenlampton]

One thing to note, the Backdrop CMS Hooks section of the documentation uses hook_help() as an example. I think that is why my team thought hook_help() was still implemented.

Hah! I'm surprised nobody else noticed this :) We'll update it shortly!

[rbargerhuff]

Hey, it happens! Haha look at me... I missed the word Drupal in installation intructions that were specific to Backdrop!!

[rbargerhuff]

Jen, question... since the Backdrop community has named bee as the replacement for drush, should drush-related files be removed from the module?

[jenlampton]

@rbargerhuff I've been leaving the drush related files in modules, because I still use drush. It's up to you!

[rbargerhuff]

The CAS repository has been successfully transferred to its new home: https://github.com/backdrop-contrib/cas