Jacob,

Nice little module. Thanks! Attached is a patch with some suggestions:

  1. fix style to pass coder review, most notably spaces before parenthesis and no spaces before and after quoted strings .' '.
  2. Add $Id$ to .module
  3. Remove package from .info - this is reserved for modules that actually ship together, unless the developers of both modules agree otherwise
  4. use $Name$ in .info version - actually, I'm not sure that this is really good or desirable, but most people do it. The project.module packaging system replaces this when creating the tar files, so the name gets replaced only when checked out using CVS.
  5. Removed extraneous html formatting in hook_help - did you really want this for some reason?
  6. Removed empty watchdog line in adminrole_update_perms - if you want to write a watchdog entry, that's fine, but please fill in the module name and use more descriptive text
  7. Remove "continue" statement in adminrole_update_perms - since there were no statements below this, it is unnecessary
  8. Remove array merging of drupal_set_message - I'm not sure why you think you needed this. I looked at the drupal_set_message code and it appeared to append messages already. I also tested it and this does indeed seem to be how it works.
  9. Simplify logic in admin_form_alter
CommentFileSizeAuthor
adminrole.patch4.8 KBdouggreen

Comments

douggreen’s picture

douggreen commented

I also removed adminrole_system_modules_form_submit. Was this for debugging only?

function adminrole_system_modules_form_submit($form_id,$form_values) {
  dpr($form_values);
  die();
}
JacobSingh’s picture

JacobSingh commented

Thanks doug, I've released it with these changes.

JacobSingh’s picture

JacobSingh commented
Status: Needs review » Fixed
Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)