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.
Comment | File | Size | Author |
---|---|---|---|
#6 | remove-unused-use-statement-2542700-6.patch | 733.08 KB | Sharique |
#1 | 2542700-1.patch | 452 bytes | joshi.rohit100 |
Comments
Comment #1
joshi.rohit100Removed. Lets see..
Comment #2
tlyngej CreditAttribution: tlyngej at Annertech commentedComment #3
tlyngej CreditAttribution: tlyngej at Annertech commentedComment #4
tlyngej CreditAttribution: tlyngej at Annertech commentedComment #5
tlyngej CreditAttribution: tlyngej at Annertech commentedComment #6
Sharique CreditAttribution: Sharique as a volunteer commentedI build this patch using phpstorm's optimise import, it is possible there might be still some files have unused import.
Comment #7
cilefen CreditAttribution: cilefen commentedComment #8
tlyngej CreditAttribution: tlyngej at Annertech commentedSharique, 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.
Comment #10
pfrenssenI 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.
Comment #11
tlyngej CreditAttribution: tlyngej at Annertech commentedComment #12
cilefen CreditAttribution: cilefen commentedRe #10 - +1 on postponing this. We need help on #2474049: [meta] Major issue triage and criticals right now more than we need this cleanup.
Comment #13
tlyngej CreditAttribution: tlyngej at Annertech commentedAs mentioned before, this started out as a pretty small trivial task :-) It all of a sudden just grew out of my hands...
Comment #14
tlyngej CreditAttribution: tlyngej at Annertech commentedThis might not be a novice task after all.
Comment #15
Sharique CreditAttribution: Sharique as a volunteer commented@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.
Comment #16
tlyngej CreditAttribution: tlyngej at Annertech commented@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.
Comment #17
Mile23This 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.