Bug 17671

Summary: Remove unused variables in Reserves.pm
Product: Koha Reporter: Petter Goksøyr Åsen <boutrosboutrosboutros>
Component: CirculationAssignee: Petter Goksøyr Åsen <boutrosboutrosboutros>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: trivial    
Priority: P5 - low CC: gmcharlt, josef.moravec, katrin.fischer, kyle.m.hall, kyle, m.de.rooy
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: remove unused variables
remove unused variables
Bug 17671: Remove unused variables in C4::Reserves

Description Petter Goksøyr Åsen 2016-11-23 10:11:36 UTC
I was reading through Reserves.pm trying to figure out a bug - found some unused variables instead.
Comment 1 Petter Goksøyr Åsen 2016-11-23 10:12:51 UTC
Created attachment 57726 [details] [review]
remove unused variables
Comment 2 Josef Moravec 2016-11-28 09:29:08 UTC
Please, provide a test plan (even if it's some brief and obvious)
Comment 3 Petter Goksøyr Åsen 2016-11-28 10:31:36 UTC
Test plan:
Visually verify that the removed variables are not referred to in the the current scope (sub).

Variables with identical names might be used in other subroutines in the file, so grepping for them will give you some false positives.
Comment 4 Alex Buckley 2016-11-30 23:47:52 UTC
Created attachment 57850 [details] [review]
remove unused variables

https://bugs.koha-community.org/show_bug.cgi?id=17671
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 5 Marcel de Rooy 2016-12-02 07:46:53 UTC
(In reply to Petter Goksøyr Åsen from comment #3)
> Test plan:
> Visually verify that the removed variables are not referred to in the the
> current scope (sub).
Reading the code is not a test plan, sir.
Check if adding a reserve still works, would be.
But obviously, I will pass QA on these trivial changes. No problem.
Koha::Old::Hold->new is hilarious btw.
Comment 6 Marcel de Rooy 2016-12-02 07:49:09 UTC
Created attachment 57883 [details] [review]
Bug 17671: Remove unused variables in C4::Reserves

I was reading through Reserves.pm trying to figure out a bug - found some
unused variables instead.

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Marcel de Rooy 2016-12-02 07:50:59 UTC
Petter,
Please add "Bug [number]:" as prefix on the first line of your commit message.
And add a commit message :)
Also fill the Assignee on Bugzilla please.
Comment 8 Petter Goksøyr Åsen 2016-12-02 10:04:23 UTC
> Reading the code is not a test plan, sir.
> Check if adding a reserve still works, would be.

Apologies! You're right of course.

> Please add "Bug [number]:" as prefix on the first line of your commit message.
> And add a commit message :)
> Also fill the Assignee on Bugzilla please.

Apologies again, I will make sure to do this next time:)

Thanks!
Comment 9 Kyle M Hall 2016-12-02 10:27:08 UTC
Pushed to master for 17.05, thanks Petter!
Comment 10 Katrin Fischer 2016-12-04 21:40:55 UTC
Decided against backporting this to stable - not a bug fix.