Bug 11123

Summary: 2 routines are unused in C4::Account
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: 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:
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
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!