Bug 10426

Summary: Remove unused sub GetCcodes from C4::Koha
Product: Koha Reporter: Colin Campbell <colin.campbell>
Component: Architecture, internals, and plumbingAssignee: Colin Campbell <colin.campbell>
Status: CLOSED FIXED QA Contact:
Severity: enhancement    
Priority: P5 - low CC: gmcharlt, kyle.m.hall, srdjan
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 10539, 10771    
Bug Blocks:    
Attachments: proposed patch
Bug 10426 Remove unused sub GetCcodes from Koha.pm
Everything checks out. Still needs dependencies to pass qa before it

Description Colin Campbell 2013-06-06 10:35:52 UTC
Only reference to routine is in a comment. Delete it Lets not waste time maintaining unused code (and I can see at least 3 things that could be fixed in it, if we were)
Comment 1 Colin Campbell 2013-06-06 10:52:19 UTC Comment hidden (obsolete)
Comment 2 Srdjan Jankovic 2013-06-07 04:41:33 UTC
Needs removing from t/db_dependent/lib/KohaTest/Koha.pm as well I guess.
Comment 3 Kyle M Hall 2013-06-07 12:47:49 UTC
In addition to Srdjan's comment, this sub is used in misc/cronjobs/stats/monthly_circulation_statistics.pl

To remove the GetCcodes sub, this script will need to be refactored or deleted.
Comment 4 Galen Charlton 2013-08-20 19:04:05 UTC
(In reply to Kyle M Hall from comment #3)
> In addition to Srdjan's comment, this sub is used in
> misc/cronjobs/stats/monthly_circulation_statistics.pl
> 
> To remove the GetCcodes sub, this script will need to be refactored or
> deleted.

This may not be a long term blocker.  See bug 10771.
Comment 5 Galen Charlton 2013-08-20 19:05:20 UTC
(In reply to Srdjan Jankovic from comment #2)
> Needs removing from t/db_dependent/lib/KohaTest/Koha.pm as well I guess.

Yes, but not necessarily a blocker overall: see bug 10539.
Comment 6 Galen Charlton 2013-08-20 19:07:22 UTC
On balance, I think the issues raised by Srdjan and Kyle need not block testing and signoff of the proposed patch, although the dependent bugs will block pushing to master.  Setting to needs signoff.
Comment 7 Srdjan Jankovic 2013-08-21 00:53:52 UTC Comment hidden (obsolete)
Comment 8 Kyle M Hall 2013-08-30 12:36:03 UTC
Created attachment 20736 [details] [review]
Everything checks out. Still needs dependencies to pass qa before it

could be pushed though.
Comment 9 Galen Charlton 2013-09-05 15:36:41 UTC
One of the dependent bugs has been pushed to master, leaving bug 10771, which needs signoff.  I encourage anybody interested in seeing this patch get pushed to review the patches for 10771.
Comment 10 Galen Charlton 2013-09-06 18:28:34 UTC
Pushed to master.  Thanks, Colin!