Full report is available here: http://pareview.sh/pareview/httpgitdrupalorgprojectsimpleaccessgit-7x-2x.

  • Mostly added function docs, I had to guess quite a bit so feel free to use better descriptions :)
  • Caught one stray dpm() call.
  • Renamed 2 variables to use the simple_access_ namespace, instead of sa_.
  • Removed some unused variables. In simple_access_profile_form() use $profile['pid'] where $pid was not defined.

Patch attached!

CommentFileSizeAuthor
#4 pareview-3.patch38.24 KBkscheirer
#2 pareview-2.patch37.44 KBkscheirer
pareview-1.patch37.44 KBkscheirer
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

kscheirer’s picture

kscheirer
Primary language English
Location Vallejo
CreditAttribution: kscheirer commented
Issue summary: View changes
Status: Active » Needs review
kscheirer’s picture

kscheirer
Primary language English
Location Vallejo
CreditAttribution: kscheirer commented
FileSize
pareview-2.patch37.44 KB

I mistakenly thought there was a hook_node_save(). This patch doesn't change that function's docblock.

gordon’s picture

gordon CreditAttribution: gordon commented
Status: Needs review » Needs work

This is great, but because there is a change in variable names there needs to be an update script which will rename these for anyone upgrading.

Thanks again.

kscheirer’s picture

kscheirer
Primary language English
Location Vallejo
CreditAttribution: kscheirer commented
Status: Needs work » Needs review
FileSize
pareview-3.patch38.24 KB

Doh, sorry about that! Updated patch with a hook_update.

gordon’s picture

gordon CreditAttribution: gordon commented
Status: Needs review » Fixed

Thanks I have committed this to dev

Status: Fixed » Closed (fixed)

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

  • Commit 4f1331f on 7.x-2.x, 8.x-3.x by gordon: 
    Issue #2133527 by kscheirer: [tsm] Code style cleanup from pareview.sh.