Bug 11123 - 2 routines are unused in C4::Account
Summary: 2 routines are unused in C4::Account
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-10-23 11:41 UTC by Jonathan Druart
Modified: 2014-05-26 21:04 UTC (History)
3 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 11123: 2 routines are unused in C4::Account (7.20 KB, patch)
2013-10-23 11:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED OFF] Bug 11123: 2 routines are unused in C4::Account (7.32 KB, patch)
2013-10-23 15:11 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 11123: 2 routines are unused in C4::Account (7.42 KB, patch)
2013-10-24 11:22 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2013-10-23 11:41:14 UTC
C4::Accounts::fixcredit and C4::Accounts::refund are marked as deprecated and are not used. They can be removed.
Comment 1 Jonathan Druart 2013-10-23 11:44:29 UTC Comment hidden (obsolete)
Comment 2 Mark Tompsett 2013-10-23 15:07:48 UTC
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.
Comment 3 Mark Tompsett 2013-10-23 15:11:16 UTC Comment hidden (obsolete)
Comment 4 Marcel de Rooy 2013-10-24 11:22:53 UTC
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>
Comment 5 Galen Charlton 2013-10-24 16:57:42 UTC
Pushed to master.  Thanks, Jonathan!