This started as a simple issue with a few removal suggestions for unused use statements. Turned out that there is a lot of those in Core.

We've found a tool that might do this change in on go.

Fabien Potencier's PHP-CS-Fixer

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joshi.rohit100’s picture

Status: Needs work » Needs review
FileSize
452 bytes

Removed. Lets see..

tlyngej’s picture

Title: Unused use statements in Core/lib » Unused use statements in Core/module
tlyngej’s picture

Issue summary: View changes
Status: Needs review » Needs work
tlyngej’s picture

Issue summary: View changes
tlyngej’s picture

Issue summary: View changes
Sharique’s picture

Status: Needs work » Needs review
FileSize
733.08 KB

I build this patch using phpstorm's optimise import, it is possible there might be still some files have unused import.

cilefen’s picture

tlyngej’s picture

Sharique, I see a lot of stuff being moved around in that patch? Also in files not being mentioned in the description. Would you mind elaborate what you've done?

Should ideally only contain removed lines.

Status: Needs review » Needs work

The last submitted patch, 6: remove-unused-use-statement-2542700-6.patch, failed testing.

pfrenssen’s picture

I haven't looked at the whole 700kb but the patch in #6 looks good. It also puts the use statements in alphabetical order, this is something that is not a hard requirement but it is generally considered to be a good thing. If we *are* touching all the use statements then we can just as well put them in the right order.

Now this is something that will indeed invalidate a lot of patches, so this is better postponed until we are out of beta. Once we have a release candidate out and the dust is settling a bit this would be a nice improvement, a nice cleanup before cutting 8.0.0.

It is very good news that this can be handled in a reliable and automated way using PHPStorm.

We should look at the failures in migrate_drupal.

tlyngej’s picture

Issue summary: View changes
cilefen’s picture

Re #10 - +1 on postponing this. We need help on #2474049: [meta] Major issue triage and criticals right now more than we need this cleanup.

tlyngej’s picture

As mentioned before, this started out as a pretty small trivial task :-) It all of a sudden just grew out of my hands...

tlyngej’s picture

Issue tags: -Novice

This might not be a novice task after all.

Sharique’s picture

@tlyngej Agree that patch should only contain removed lines, but if we are going to use automated tools, then we have to accept some trade offs.
@pfrenssen will look into failing tests.

tlyngej’s picture

@Sharique It might be meaningless to create a patch for this now. There are to many parts moving, and this patch will require all other waiting patches to be re-rolled. @cilefen mentioned that we should wait until after RC release, in which I agree. This issue is touching pretty much every module in core. Not sure about lib's. haven't checked those.

Mile23’s picture

Status: Needs work » Closed (duplicate)
Related issues: +#2533800: Remove all unused use statements from core

This is a duplicate of #2533800: Remove all unused use statements from core, which is filed by a maintainer. Apparently the schedule is to do this step during RC, when there won't be as many conflicts.