Part of meta-issue #2002650: [meta, no patch] improve maintainability by removing unused local variables

File /core/modules/xmlrpc/xmlrpc.server.inc

Line 216: Unused local variable $return_type

CommentFileSizeAuthor
#8 2080709-remove-unused-variables-8.patch751 bytesmikemiles86
#3 2080709-remove-unused-variables-3.patch673 byteschertzog
#1 2080709-remove-unused-variables.patch643 byteschertzog
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chertzog’s picture

chertzog
he/him
Primary language English
Location Harrisburg, PA, USA
CreditAttribution: chertzog commented
Status: Active » Needs review
FileSize
2080709-remove-unused-variables.patch643 bytes

Status: Needs review » Needs work

The last submitted patch, 2080709-remove-unused-variables.patch, failed testing.

chertzog’s picture

chertzog
he/him
Primary language English
Location Harrisburg, PA, USA
CreditAttribution: chertzog commented
Status: Needs work » Needs review
FileSize
2080709-remove-unused-variables-3.patch673 bytes

Try again.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave commented

If we are to remove $return_type here I think a comment may be needed as to why we are calling array_shift(), as it is not immediately obvious.

areke’s picture

areke CreditAttribution: areke commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

This looks good. Thank you!

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Needs work

Yeah, a comment here would be good.

mikemiles86’s picture

mikemiles86
he/him
CreditAttribution: mikemiles86 commented
Assigned: Unassigned » mikemiles86

I'll take a stab at providing a helpful comment.

mikemiles86’s picture

mikemiles86
he/him
CreditAttribution: mikemiles86 commented
Status: Needs work » Needs review
FileSize
2080709-remove-unused-variables-8.patch751 bytes

Added comment about why the $signature array is being shifted.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Title: Remove Unused local variable $return_type from /core/modules/xmlrpc/xmlrpc.server.inc » Remove unused local variables from the XMLRPC module
Priority: Normal » Minor

Let's also check the rest of the module and confirm that there are no other unused local variables.

mikemiles86’s picture

mikemiles86
he/him
CreditAttribution: mikemiles86 commented
Assigned: mikemiles86 » Unassigned
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Component: other » xml-rpc system
mikemiles86’s picture

mikemiles86
he/him
CreditAttribution: mikemiles86 commented
Assigned: Unassigned » mikemiles86

Checking for additional unused variables.

mikemiles86’s picture

mikemiles86
he/him
CreditAttribution: mikemiles86 commented
Status: Needs review » Needs work
mikemiles86’s picture

mikemiles86
he/him
CreditAttribution: mikemiles86 commented
Assigned: mikemiles86 » Unassigned
Status: Needs work » Needs review

After applying the patch, and running inspector in PHPStorm no other unused variables were detected.

drewbyist’s picture

drewbyist CreditAttribution: drewbyist commented
Assigned: Unassigned » drewbyist
drewbyist’s picture

drewbyist CreditAttribution: drewbyist commented
Assigned: drewbyist » Unassigned
Status: Needs review » Reviewed & tested by the community

Tested site with XMLRPC enabled, appears good.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

That works, thanks!

Committed and pushed to 8.x.

Status: Fixed » Closed (fixed)

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