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

Comments

chertzog’s picture

chertzog
he/him
Primary language English
Location Harrisburg, PA, USA
commented
Status: Active » Needs review
StatusFileSize
new 2080717-remove-unused-variables.patch994 bytes
enhdless’s picture

enhdless commented

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

areke’s picture

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

areke commented
Status: Needs work » Needs review
StatusFileSize
new 2080717-4.patch1.45 KB

Re-rolled.

deneo2’s picture

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 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 commented
Status: Reviewed & tested by the community » Needs review
Issue tags: -Needs reroll
StatusFileSize
new 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 commented
Component: other » config.module
areke’s picture

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 🇨🇦
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.