Hi there,

love the module, but I had some issues grocking where the menu entries were and what is needed to enable this module.

See #1.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Kiphaas7’s picture

Status: Active » Needs review
FileSize
10.46 KB

Attached patch tries to:

  • Fix some very minor typo's in the inline documentation
  • Change around the menu entries to be local tasks (so they appear together)
  • Expand explanation of the "enabled state" of the module, adding that it does not work if the site is either in maintenance mode or if "vanilla aggregation" is not enabled: especially the last one was a huge WTF for me, I assumed this module would take over aggregation and thus I'd actually need to disable vanilla aggregation. Something that only cleared up as soon as I looked in the source code
  • Group admin settings per subject (in 3 fieldsets)

Pretty straightforward patch, though looking forward to any critique on it!

nagba’s picture

Thanks, I will take a look at it in the coming days. Better explanation is always welcomed, a fresh set of eyes helps a lot. :)

Kiphaas7’s picture

Actually missed a t() call in an addition, updated patch.

Glad to be of any help :).

Kiphaas7’s picture

By the way: if the patch is too big to review at once, I actually made all the changes in my sandbox:
http://drupal.org/node/1732740/commits

Separates the above patch in 5 commits.

Kiphaas7’s picture

Status: Needs review » Needs work

+ '!aggregate' => l(t('aggregate CSS and/or JS files'), 'admin/config/development/performance', array('attributes' => array('title' => t('Enable or disable page caching for anonymous users and set CSS and JS bandwidth optimization options.')))),

Has a trailing whitespace :/. Will fix later today.

Kiphaas7’s picture

Status: Needs work » Needs review
FileSize
10.36 KB
Kiphaas7’s picture

FileSize
10.84 KB

This is starting to look like spamming, but I actually honestly forgot to update the .info file...

nagba’s picture

Status: Needs review » Reviewed & tested by the community

looks good to me

nagba’s picture

Status: Reviewed & tested by the community » Fixed

I've commited your code, hopefully soon the dev release will update itself and i'll be able to create a new prod release as well.

Kiphaas7’s picture

thanks!

nagba’s picture

Status: Fixed » Closed (fixed)

Closing issue as the code is now on both dev and stable releases.

nagba’s picture

Issue summary: View changes

clarify OP