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.
Remove unused imports in code base.
Comment | File | Size | Author |
---|---|---|---|
#14 | paragraphs-remove_unused_imports-2643752-14.patch | 12.68 KB | petermallett |
| |||
#12 | interdiff-2643752-6-12.txt | 14.68 KB | petermallett |
#12 | paragraphs-remove-deprecated-2643752-12.patch | 11.26 KB | petermallett |
#6 | interdiff-2643752-2-6.txt | 762 bytes | heykarthikwithu |
#6 | 2643752-6.patch | 7.52 KB | heykarthikwithu |
Comments
Comment #2
heykarthikwithuComment #6
heykarthikwithuComment #10
heykarthikwithuComment #11
miro_dietikerThank you for cleaning up the code.
It's important that with cleanup commits, we are not breaking other pending patches. And as such, if we commit a cleanup, it should cover all similar aspects in one commit.
I see you caught all unused use declaration.
From quickly running code analysis in PHPStorm, i see that there are other deprecated remaining things such as: (still) EntityManager, SelectionBase, entity_view()
Let's clean them up in the same cycle.
Hm, this method is not declared as static! We should not call it statically!
Comment #12
petermallett CreditAttribution: petermallett at Mediacurrent commentedI couldn't get this patch to apply to the current 8.x-1.x -- mostly because a lot of the issues were fixed in other updates already. So I went about making a new version of the patch to remove unused use statements & update the remaining deprecated method calls (also corrected the use of FileSystem::realpath()).
Note: tests are failing but I reviewed them and none of the problems look like they're related to these changes.
Comment #13
miro_dietikerThank you for the update.
We have a Paragraphs Sprint going on until thursday. Code clean-up commits contain the risk to make other patches not apply anymore. Since there is a high amount of other activity and i don't want to create collateral damage, i will postpone committing code-clean-ups until the sprint ended.
Comment #14
petermallett CreditAttribution: petermallett at Mediacurrent commentedCircling back after a long time to see if we can get this one moving again.
Since it is highly unlikely that a monolithic cleanup patch will ever be applied because it will always conflict with other patches in flight, I propose that we narrow the scope of this issue and break out smaller issues for working on replacing deprecated methods.
Here's a patch to do a smaller cleanup pass: just removes unused imports (and a few minor whitespace tweaks). This is less likely to conflict with ongoing work in other issues.
phpcs used to identify cleanup:
phpcs --standard=Drupal --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md --sniffs=Drupal.Classes.UnusedUseStatement,Squiz.WhiteSpace.SuperfluousWhitespace ./
Comment #15
BerdirSeems like a useful split. testbot also reports 37 fewer coding style issues. I wish they would display an icon directly in the issue, I never look into the detail page.
Comment #16
miro_dietikerCommitted, thank you.