search

backdrop-contrib / domain

A domain-based access control system for Backdrop CMS
https://backdropcms.org/project/domain
3 stars 5 forks source link

Update comments in settings.inc file. #37

Closed izmeez closed 1 week ago

izmeez commented 4 weeks ago

This issue is a follow-up to https://github.com/backdrop-contrib/domain/issues/25

The comments in the settings.inc file need to be updated for Backdrop. PR to follow.

izmeez commented 4 weeks ago

The PR needs review. It includes comment updates for Backdrop, clarity, and phpcs.

izmeez commented 4 weeks ago

I wonder if the link to the drupal issue queue, https://www.drupal.org/node/1559486, should be a link to the comment adding try and catch, https://www.drupal.org/project/domain/issues/1559486#comment-10188570 that allows the settings.inc file to be included in the settings.php file even before Backdrop is installed. There are other comments in the thread that are no longer relevant.

stpaultim commented 1 week ago

@yorkshire-pudding This PR "Works for me"

Does someone else want to merge it and close this issue.

NOTE: It's late and I did not look super closely at it. But, skimming through it, the changes all seem very sensible and low risk.

izmeez commented 1 week ago

Should the link be updated as described in comment above https://github.com/backdrop-contrib/domain/issues/37#issuecomment-2433155454 ?

izmeez commented 1 week ago

@yorkshire-pudding Thank you for reviewing the PR. Changes made as per suggestions.