Removal of unused variables in the code base.

CommentFileSizeAuthor
#9 acl-2612464-9-D8.patch492 bytessalvis
#4 interdiff-2612464-2-4.txt294 bytesheykarthikwithu
#4 2612464-4.patch1015 bytesheykarthikwithu
#2 2612464-2.patch975 bytesheykarthikwithu
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

heykarthikwithu created an issue. See original summary.

heykarthikwithu’s picture

heykarthikwithu
Primary language English
Location Bengaluru 🌍
CreditAttribution: heykarthikwithu as a volunteer and at Valuebound commented
Assigned: heykarthikwithu » Unassigned
Status: Active » Needs review
FileSize
2612464-2.patch975 bytes

Removed the unused variables $button_name and $path.

salvis’s picture

salvis
he/his
CreditAttribution: salvis commented
Status: Needs review » Needs work

Have you checked whether these also appear in the D8 version? The latest version always needs to go first.

+++ b/acl.module
@@ -317,7 +317,6 @@ function _acl_module_load_include($type) {
-    $path = module_load_include($type, 'acl');

module_load_include() has a side effect. Removing the call probably breaks ACL.

We may be able to save the call to drupal_get_path() depending on what we get back from module_load_include().

Blindly going through the code base and mechanically removing everything flagged by some code inspection tool is a waste of maintainer time.

heykarthikwithu’s picture

heykarthikwithu
Primary language English
Location Bengaluru 🌍
CreditAttribution: heykarthikwithu as a volunteer and at Valuebound commented
Status: Needs work » Needs review
FileSize
2612464-4.patch1015 bytes
interdiff-2612464-2-4.txt294 bytes

rerolled the patch.

1. $button_name = 'acl_' . $form['acl_id']['#value'];
$button_name is no more used in the code base, so this can be removed.

2. $path = module_load_include($type, 'acl');
$path variable is no more used in the code base. so this is replaced with
module_load_include($type, 'acl');

So this works fine :)

Elin Yordanov’s picture

Elin Yordanov CreditAttribution: Elin Yordanov commented
Status: Needs review » Reviewed & tested by the community

This is a reasonable patch. It makes the code cleaner and has no impact on the functionality.

  • salvis committed f9cc786 on 7.x-1.x 
    #2612464 by heykarthikwithu, salvis: Remove of unused variables in the...
salvis’s picture

salvis
he/his
CreditAttribution: salvis commented
Status: Reviewed & tested by the community » Fixed

Thank you, heykarthikwithu and Елин Й.!

Status: Fixed » Closed (fixed)

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

salvis’s picture

salvis
he/his
CreditAttribution: salvis commented
Version: 7.x-1.x-dev » 8.x-1.x-dev
Status: Closed (fixed) » Needs review
FileSize
acl-2612464-9-D8.patch492 bytes

Well, I guess I misinterpreted #4.

  • salvis committed 10e4bbee on 8.x-1.x 
    Issue #2612464 by salvis: Remove unused variable.
salvis’s picture

salvis
he/his
CreditAttribution: salvis commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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