Allows admin or module to define paths to be included or excluded from purl's outbound url rewrites on a global or provider level.

This module is useful if certain purl providers are only relevant for specific pages and not others.

This module can function in two modes, "Include" or "Exclude". In include mode, only paths defined in the admin or hook_purl_active_paths will be rewritten by purl in outbound links. In exclude mode, these paths will be excluded from rewrites.

I have found nothing like this module.

Link: http://drupal.org/sandbox/emosbaugh/1127314

Comments

ralt’s picture

ralt commented

Hello,

Here is a quick review :

  • Add a README.txt file
  • In your .install file, take off this $Id$ on the same line as the php opening tag, and add a blank end line
  • Still in your .install file, please add variable_del() for every persistent variable your module might set
  • In your .admin.inc file
    • Take this $Id$ off.
    • You're setting a LOT of persistent variables at line 40. You should think about having your own table.
    • Add a blank end line
  • In your .api.php file, take off this $Id$
  • In your .module file
    • Take this $Id$ off
    • Add a comment to _purl_active_paths_invoke_all()
    • Add a blank end line

Otherwise, the module is really good!

Have a nice day :-)

ralt’s picture

ralt commented
Status: Needs review » Needs work
emosbaugh’s picture

emosbaugh commented
Status: Needs work » Needs review

You're setting a LOT of persistent variables at line 40. You should think about having your own table.
- I can't see any site setting very many persistant variables here. It is only one per purl provider, not provider id. I think it would be overkill to create my own table. I will if you still think it is necessary.

Add a blank end line
- I believe I have one or I do not understand this request. Please explain.

I have fixed all other issues. Thank you for the feedback.

lslinnet’s picture

lslinnet commented
Status: Needs review » Reviewed & tested by the community

I don't see any harm in settings this in variables, it's only for modules that implemenets the hook_purl_provider().

the "if" structure on line 37 of in the .module file could use a pair of {} around it.

except for that it looks good, in my opinion it's ready to be upgrade to a full module status.

emosbaugh’s picture

emosbaugh commented

done. thanks. hope this module is useful for others.

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented
Status: Reviewed & tested by the community » Fixed

Git vetted User access granted. Thanks for your contributions, and future contributions, and for your patience during this process! Please make sure to help review other applications.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.