Bug 17671 - Remove unused variables in Reserves.pm
Summary: Remove unused variables in Reserves.pm
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low trivial (vote)
Assignee: Petter Goksøyr Åsen
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-11-23 10:11 UTC by Petter Goksøyr Åsen
Modified: 2017-06-14 22:11 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
remove unused variables (2.21 KB, patch)
2016-11-23 10:12 UTC, Petter Goksøyr Åsen
Details | Diff | Splinter Review
remove unused variables (2.27 KB, patch)
2016-11-30 23:47 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 17671: Remove unused variables in C4::Reserves (2.44 KB, patch)
2016-12-02 07:49 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 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.