We have 'use' statements that are useless. We could remove them.
Created attachment 135804 [details] [review] Bug 30926: [DO NOT PUSH] Yet another script
Created attachment 135805 [details] [review] Bug 30926: Remove unnecessary use statements This is the result of: perl remove_unused_import.pl **/*.pl **/*.pm svc/* svc/**/*
Created attachment 135806 [details] [review] Bug 30926: Manual fixes Wrong occurrences caught by the script
Very boring job now to confirm the script is not buggy. I don't think I am going to work more on this one.
Created attachment 136250 [details] [review] Bug 30926: Remove unnecessary use statements This is the result of (using a zsh shell): perl remove_unused_import.pl **/*.pl **/*.pm svc/* svc/**/*
Created attachment 136251 [details] [review] Bug 30926: Manual fixes Wrong occurrences caught by the script
Tomas: Is this something that you would be willing to push quickly? In that case I might have a look soon,,
(In reply to Marcel de Rooy from comment #7) > Tomas: Is this something that you would be willing to push quickly? > In that case I might have a look soon,, yes...
(In reply to Jonathan Druart from comment #8) > (In reply to Marcel de Rooy from comment #7) > > Tomas: Is this something that you would be willing to push quickly? > > In that case I might have a look soon,, > > yes... Tomas never responded
(In reply to Marcel de Rooy from comment #9) > (In reply to Jonathan Druart from comment #8) > > (In reply to Marcel de Rooy from comment #7) > > > Tomas: Is this something that you would be willing to push quickly? > > > In that case I might have a look soon,, > > > > yes... > > Tomas never responded There are too many things to look after, all the time. I've been pushing as fast as possible this kind of patch sets. If you are willing to work in this a bit more, I'm all in. Just DM me when ready so I add it to my priorities. Best regards
Created attachment 138795 [details] [review] Bug 30926: Remove unnecessary use statements This is the result of: perl remove_unused_import.pl **/*.pl **/*.pm svc/* svc/**/*
No test plan, it's expected. If you don't know what to do here, pick another bug ;)
The script looks fairly simple and readable.. feels correct. I'm no expert on imports however... The results seem reasonable on all tests I currently performed.. I think this might be a 'push and fix the mess' type bug.. it feels good to go to me, but I wouldn't be at all surprised if there are some random bugs deep in there.. I'm happy to monitor and fix the resulting issues in Jonathans absence once he's out later this month if it helps.
Did some testing, even though this doesn't currently apply on top of master. However seems like the script might be a bit too eager with the removals: > Compilation failed in require at /kohadevbox/koha/Koha/ItemTypes.pm line 23. I get this among other failures. In ItemTypes.pm C4::Languages is used but this patch removed the use statement. After this patch has been made there has been no changes to ItemTypes.pm so I think it is indeed an issue with the script that generates this patch. Another corner case not handled by the script: C4/SIP/SIPServer.pm: C4::SIP::Trapper import is used by the tie *STDERR, "C4::SIP::Trapper"; statement.
I'd love to see this pursued early next cycle..