Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Part of meta-issue #2002650: [meta, no patch] improve maintainability by removing unused local variables
File /core/modules/image/image.module
Line 251: Unused local variable $style_name
Comment | File | Size | Author |
---|---|---|---|
#10 | 2080391-remove-unused-variables-new.patch | 9.25 KB | johnmcc |
#10 | interdiff.txt | 2.63 KB | johnmcc |
#7 | 2080391-remove-unused-variables.patch | 842 bytes | johnmcc |
#5 | drupal8.other_.2080391-5.patch | 510 bytes | JeroenT |
#2 | 2080391-remove-unused-variables.patch | 8.91 KB | chertzog |
Comments
Comment #1
chertzogClosing the following as duplicates.
#2080093: Remove Unused local variable $form from /core/modules/system/lib/Drupal/system/Tests/Form/FormTest.php
#2081147: Remove Unused local variable $links from /core/modules/content_translation/content_translation.pages.inc
#2080051: Remove Unused local variable $default from /core/lib/Drupal/Component/Gettext/PoHeader.php
#2080091: Remove Unused local variable $name_field from /core/modules/system/tests/modules/database_test/database_test.module
#2080533: Remove Unused local variable $value from /core/modules/options/options.module
#2080079: Remove Unused local variable $display_title from /core/modules/views_ui/lib/Drupal/views_ui/ViewEditFormController.php
#2080545: Remove Unused local variable $base_path from /core/modules/overlay/overlay.module
#2080101: Remove Unused local variable $entities from /core/modules/system/lib/Drupal/system/Tests/Entity/EntityRevisionsTest.php
Comment #2
chertzogComment #4
BladeduThis is doing 3 array_shift in a row. I think it would be better to use array_slice() passing as offset 0 and as length 3.
I think that this part shouldn't be touched, because the form_test_validate_required_form_no_title() function does not get form as reference and $form is still used later on.
Comment #5
JeroenTRemoved the unused local variable $style_name.
Comment #6
BladeduThis last patch doesn't look good to me.
Removing entirely this part will break the hook_implementation because the style name is not shifted out from the args anymore.
Comment #7
johnmcc CreditAttribution: johnmcc commentedI agree with #6 - we still need to remove the style name.
Here's a patch that takes into account #3, using array_slice rather than three array_shifts. (My first patch, be gentle!)
Comment #8
johnmcc CreditAttribution: johnmcc commentedComment #9
BladeduThe usage of array_slice is ok, but if you look at patch #2, there were many others fixes which are omitted in this patch. Please re-submit it, and make sure to fix patch #2 with my second remark in comment #4.
Comment #10
johnmcc CreditAttribution: johnmcc commentedThanks for the feedback! I've reworked patch #2. Couple of things to note:
Comment #11
BladeduI'll take a second look later just to be sure, but so far it looks pretty good :)
Comment #12
BladeduThank you for the patch :)
Comment #13
webchickCommitted and pushed to 8.x. Thanks!
Comment #15
YesCT CreditAttribution: YesCT commented#2080051: Remove Unused local variable $default from /core/lib/Drupal/Component/Gettext/PoHeader.php was closed as a duplicate of this issue, but this issue did not remove this unused var:
Line 231: Unused local variable $default
why?
see #2079863: Remove Unused local variable from /core/lib/Drupal/Component Utility/Crypt.php Transliteration/PHPTransliteration.php Utility/UserAgent.php Gettext/PoHeader.php which has a patch to remove that one.