Summary: | 2 routines are unused in C4::Account | ||
---|---|---|---|
Product: | Koha | Reporter: | Jonathan Druart <jonathan.druart> |
Component: | Architecture, internals, and plumbing | Assignee: | Jonathan Druart <jonathan.druart> |
Status: | CLOSED FIXED | QA Contact: | Marcel de Rooy <m.de.rooy> |
Severity: | normal | ||
Priority: | P5 - low | CC: | gmcharlt, m.de.rooy, mtompset |
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: | ||
Circulation function: | |||
Attachments: |
Bug 11123: 2 routines are unused in C4::Account
[SIGNED OFF] Bug 11123: 2 routines are unused in C4::Account Bug 11123: 2 routines are unused in C4::Account |
Description
Jonathan Druart
2013-10-23 11:41:14 UTC
Created attachment 22308 [details] [review] Bug 11123: 2 routines are unused in C4::Account C4::Accounts::fixcredit and C4::Accounts::refund are marked as deprecated and are not used. They can be removed. Use: git grep fixcredit git rgep refund and verify these routines are not currently in used. Bonus: The module exports reconcileaccount which is not defined. The export is removed too. I noticed that a function call to the refund was commented out in C4::Accounts, having not even read the patch. I did a: git blame -L440,450 C4/Accounts.pm to discover that the commit in question was likely done in 2008. So, the second and third blocks displayed in the patches "Diff" have about 5 years old. I don't see a problem with them. I went brute force and did a: grep fixcredit `find .` And the only code found was C4::Accounts. This included: the commented line, two POD lines, and the function definition line. I see no reason not to remove fixcredit. When I did the same for refund, it was messier, so I hoped that the git grep provided was cleaner. It basically was the same thing, except piped to less. C4::Accounts has 3 lines related to the deprecated function refund. I ignored the blib and po results. There were unrelated results in the stats.screen.pl, sysprefs.sql, updatedatabase.pl, and various template files. As far as I can see, there is nothing using the refund function in C4::Accounts. There is only one reference to reconcileaccount, which is the line that is exporting it. That makes no sense, so I see no reason to prevent removal. In short, this patch is good. Created attachment 22319 [details] [review] [SIGNED OFF] Bug 11123: 2 routines are unused in C4::Account C4::Accounts::fixcredit and C4::Accounts::refund are marked as deprecated and are not used. They can be removed. Use: git grep fixcredit git grep refund and verify these routines are not currently in used. Bonus: The module exports reconcileaccount which is not defined. The export is removed too. MLT: Ran qa tool on this patch with no issues either. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Created attachment 22352 [details] [review] Bug 11123: 2 routines are unused in C4::Account C4::Accounts::fixcredit and C4::Accounts::refund are marked as deprecated and are not used. They can be removed. Use: git grep fixcredit git grep refund and verify these routines are not currently in used. Bonus: The module exports reconcileaccount which is not defined. The export is removed too. MLT: Ran qa tool on this patch with no issues either. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Pushed to master. Thanks, Jonathan! |