Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Postponed on #3052377: Drupal.Classes.UnusedUseStatement sniff is incomplete
Delete unused import
Comment | File | Size | Author |
---|---|---|---|
#25 | 3041983-25.patch | 19.89 KB | yogeshmpawar |
Comments
Comment #2
pifagorComment #3
pifagorComment #4
pifagorComment #5
volegerNice, simple cleanup. +1 for rtbc
Comment #6
alexpottThis means that Drupal.Classes.UnusedUseStatement is broken - we need to fix this in coder and then update core\s usage of coder and fix it there.
Comment #7
idebr CreditAttribution: idebr at ezCompany commentedI filed #3052377: Drupal.Classes.UnusedUseStatement sniff is incomplete in the Coder module to fix the Drupal.Classes.UnusedUseStatement sniff
Comment #8
klausiI don't understand this removal. Url is used in this file so we need the use statement?
Comment #9
pifagorBecause this file uses "\Drupal::url" and in the "use Drupal\Core\Url;" is no need.
Comment #10
klausiI don't think so - the file uses Url::fromRoute(), so we need the use statement? See https://git.drupalcode.org/project/drupal/blob/8.8.x/core/modules/migrat...
Comment #11
pifagorFixed I mistakenly viewed the previous version of the core
Comment #12
pifagorhttps://www.drupal.org/project/coder/issues/3052377 - completed
Comment #13
alexpottWe need to get coder in Drupal core updated to use the new rule - which means we need a release of coder.
Comment #14
klausiCoder 8.3.4 released, so you can upgrade here I guess?
Comment #15
alexpottWell there's also #3049433: Coding standards not running on d.o. - upgrade to coder 8.3.4 - these efforts need merging because upgrading is going to uncover this.
Comment #16
Gábor HojtsyComment #17
idebr CreditAttribution: idebr at ezCompany commentedAttached patch updates drupal/coder to 8.3.4 (3041983-17-test-only.patch)
The second patch also applies the newly reported coding standards fixes.
Comment #18
klausiLooks good, just some minor things:
Unrelated change? The PHP requirements of any package should not change with this patch?
wrong change, the opening bracket a couple lines above needs to be fixed instead.
same here.
Comment #19
yogeshmpawarComment #20
yogeshmpawarAddressed comments #18 in updated patch & added an interdiff as well.
Comment #21
klausiThanks a lot, this is almost ready, just one minor thing:
sorry, just also checked these changes and they are also unrelated and should be removed. https://github.com/bovigo/vfsStream is the new home of that package and https://github.com/mikey179/vfsStream redirects there, so we should not change it in our lock file because it is already correct.
Comment #22
idebr CreditAttribution: idebr at ezCompany commented#18.1 Drupal Core changed its PHP requirement in composer.json in #3053363: Remove support for PHP 5 in Drupal 8.8, but the composer.lock file was not updated with this change. The change is therefore correct, and will always show for any `composer update X` call. We can change this in a separate issue if you like?
Comment #23
klausiI see, totally missed the PHP 7 upgrade in core (yay!). Please do it in a separate issue so that we keep the changes here small and in scope for this change.
Comment #24
yogeshmpawarComment #25
yogeshmpawarComments addressed of @klausi from #21 & added an interdiff as well.
Comment #26
idebr CreditAttribution: idebr at ezCompany commented#23 Filed #3057575: Removal of PHP5 support is not yet reflected composer.lock to update the PHP requirement in composer.lock
Comment #27
klausiPerfect, all good to go!
Comment #28
alexpottCommitted 7e08466 and pushed to 8.8.x. Thanks!
Comment #30
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedJust a question - composer.lock file now loads coder 8.3.4, but the composer.json file still has
Is it OK to leave it like this? I know that ^8.3.2 will be satisfied by 8.3.4 but does it look odd to have the discrepancy?