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

File /core/modules/config/lib/Drupal/config/Tests/ConfigImportUITest.php

Line 41: Unused local variable $storage
Line 123: Unused local variable $active

CommentFileSizeAuthor
#8 2080717-remove-unused-variables-8.patch1003 bytesTR
#4 2080717-4.patch1.45 KBareke
#1 2080717-remove-unused-variables.patch994 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
2080717-remove-unused-variables.patch994 bytes
enhdless’s picture

enhdless CreditAttribution: enhdless commented

Tried this patch, but the result is "patch does not apply"

areke’s picture

areke CreditAttribution: areke commented
Issue summary: View changes
Status: Needs review » Needs work
Issue tags: +Needs reroll
areke’s picture

areke CreditAttribution: areke commented
Status: Needs work » Needs review
FileSize
2080717-4.patch1.45 KB

Re-rolled.

deneo2’s picture

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

The patch applies fine and solve the issue correctly. Good job!

TR’s picture

TR CreditAttribution: TR commented

1: 2080717-remove-unused-variables.patch queued for re-testing.

What's wrong with the first patch? It removes the entire line for each variable. The second patch just removes the variable but still performs a useless get() where the results aren't saved.

The last submitted patch, 1: 2080717-remove-unused-variables.patch, failed testing.

TR’s picture

TR CreditAttribution: TR commented
Status: Reviewed & tested by the community » Needs review
Issue tags: -Needs reroll
FileSize
2080717-remove-unused-variables-8.patch1003 bytes

I rebased the patch from #1 so that it applied to the current HEAD.

The patch from #4 is just wrong.

TR’s picture

TR CreditAttribution: TR commented
Component: other » config.module
areke’s picture

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

Sorry about that. Anyways, this applies and looks good. Thank you!

webchick’s picture

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

Committed and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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