The module needs to be fully updated to match the Drupal coding standards as right now there are lots of small mistakes.

CommentFileSizeAuthor
#8 optify-n2048257-8.patch6.42 KBdrupalninja99
#8 interdiff.txt1.87 KBdrupalninja99
#6 optify-n2048257-6.patch5.77 KBdrupalninja99
#6 interdiff.txt2.77 KBdrupalninja99
#5 optify-n2048257-5.patch4.08 KBdrupalninja99
#4 interdiff.txt4.7 KBdrupalninja99
#4 optify-n2048257-4.patch4.55 KBdrupalninja99
#1 optify-n2048257-1.patch7.08 KBdamienmckenna

Comments

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Active » Needs review
StatusFileSize
new optify-n2048257-1.patch7.08 KB

This fixes a number of mistakes.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

Bump.

drupalninja99’s picture

drupalninja99 commented
Status: Needs review » Fixed

These changes look good, I have committed to 7.x-1.x

drupalninja99’s picture

drupalninja99 commented
Status: Fixed » Needs review
StatusFileSize
new optify-n2048257-4.patch4.55 KB
new interdiff.txt4.7 KB

I have fixed some whitespace errors and other codesniffer type items in this patch, per comments from https://drupal.org/node/1961614#comment-7688379.

drupalninja99’s picture

drupalninja99 commented
StatusFileSize
new optify-n2048257-5.patch4.08 KB

Re-rolled after admin.inc issue merge

drupalninja99’s picture

drupalninja99 commented
StatusFileSize
new interdiff.txt2.77 KB
new optify-n2048257-6.patch5.77 KB

I also moved the configuration page to /admin/config/services/optify which I think is a better fit.

drupalninja99’s picture

drupalninja99 commented
Status: Needs review » Needs work

From https://drupal.org/node/1961614#comment-7689653

You shouldn't set a default value in optify_install(). Instead, when you retrieve the variable provide the default there, like $no_display = variable_get('optify_no_display', "admin\nadmin/*\nbatch\nnode/add*\nnode/*/*\nuser/*/*");.

drupalninja99’s picture

drupalninja99 commented
StatusFileSize
new interdiff.txt1.87 KB
new optify-n2048257-8.patch6.42 KB

Made updates from #7, attaching patch and interdiff.txt.

drupalninja99’s picture

drupalninja99 commented
Status: Needs work » Needs review
drupalninja99’s picture

drupalninja99 commented
Status: Needs review » Fixed

Committed to 7.x-1.x

Status: Fixed » Closed (fixed)

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