Bug 30926 - Remove unnecessary use statements
Summary: Remove unnecessary use statements
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 17600
Blocks:
  Show dependency treegraph
 
Reported: 2022-06-08 13:20 UTC by Jonathan Druart
Modified: 2023-10-25 14:26 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 30926: [DO NOT PUSH] Yet another script (2.03 KB, patch)
2022-06-08 13:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30926: Remove unnecessary use statements (129.55 KB, patch)
2022-06-08 13:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30926: Manual fixes (701 bytes, patch)
2022-06-08 13:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30926: Remove unnecessary use statements (216.84 KB, patch)
2022-06-17 09:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30926: Manual fixes (701 bytes, patch)
2022-06-17 09:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30926: Remove unnecessary use statements (210.24 KB, patch)
2022-08-08 08:16 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2022-06-08 13:20:23 UTC
We have 'use' statements that are useless. We could remove them.
Comment 1 Jonathan Druart 2022-06-08 13:31:30 UTC
Created attachment 135804 [details] [review]
Bug 30926: [DO NOT PUSH] Yet another script
Comment 2 Jonathan Druart 2022-06-08 13:31:35 UTC
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/**/*
Comment 3 Jonathan Druart 2022-06-08 13:31:38 UTC
Created attachment 135806 [details] [review]
Bug 30926: Manual fixes

Wrong occurrences caught by the script
Comment 4 Jonathan Druart 2022-06-08 13:32:57 UTC
Very boring job now to confirm the script is not buggy. I don't think I am going to work more on this one.
Comment 5 Jonathan Druart 2022-06-17 09:47:41 UTC
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/**/*
Comment 6 Jonathan Druart 2022-06-17 09:47:46 UTC
Created attachment 136251 [details] [review]
Bug 30926: Manual fixes

Wrong occurrences caught by the script
Comment 7 Marcel de Rooy 2022-06-17 09:51:15 UTC
Tomas: Is this something that you would be willing to push quickly?
In that case I might have a look soon,,
Comment 8 Jonathan Druart 2022-08-05 07:24:10 UTC
(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...
Comment 9 Marcel de Rooy 2022-08-05 07:28:22 UTC
(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
Comment 10 Tomás Cohen Arazi 2022-08-05 11:22:58 UTC
(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
Comment 11 Jonathan Druart 2022-08-08 08:16:41 UTC
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/**/*
Comment 12 Jonathan Druart 2022-08-08 08:17:12 UTC
No test plan, it's expected. If you don't know what to do here, pick another bug ;)
Comment 13 Martin Renvoize 2022-08-08 08:59:07 UTC
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.
Comment 14 Joonas Kylmälä 2022-09-07 19:10:50 UTC
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.
Comment 15 Martin Renvoize 2023-10-25 14:26:07 UTC
I'd love to see this pursued early next cycle..