Bug 34212 - Bug 23336 follow-up code improvements
Summary: Bug 23336 follow-up code improvements
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on: 23336
Blocks:
  Show dependency treegraph
 
Reported: 2023-07-06 06:36 UTC by Martin Renvoize
Modified: 2023-09-12 07:12 UTC (History)
1 user (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00


Attachments
Bug 34212: Replace Koha::Checkouts->find with $item->checkout (784 bytes, patch)
2023-07-06 08:09 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 34212: Replace Koha::Checkouts->find with $item->checkout (784 bytes, patch)
2023-07-07 09:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 34212: Compare strings with eq instead of == (845 bytes, patch)
2023-07-07 09:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 34212: Fix tests (1.79 KB, patch)
2023-07-07 09:55 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2023-07-06 06:36:19 UTC
Jonathan highlighted a few clean ups and improvements just after push of bug 23336. I intend to take care of them here so we don't lose them on the bug itself.
Comment 1 Martin Renvoize 2023-07-06 08:09:09 UTC
Created attachment 153084 [details] [review]
Bug 34212: Replace Koha::Checkouts->find with $item->checkout
Comment 2 Jonathan Druart 2023-07-07 08:47:03 UTC
No need to call get_from_storage here?
Comment 3 Jonathan Druart 2023-07-07 08:48:08 UTC
I don't think it's needed but I would confirm it just in case.
Comment 4 Martin Renvoize 2023-07-07 09:55:40 UTC
Created attachment 153145 [details] [review]
Bug 34212: Replace Koha::Checkouts->find with $item->checkout
Comment 5 Martin Renvoize 2023-07-07 09:55:43 UTC
Created attachment 153146 [details] [review]
Bug 34212: Compare strings with eq instead of ==

And don't ignore warnings next time ;)

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize 2023-07-07 09:55:45 UTC
Created attachment 153147 [details] [review]
Bug 34212: Fix tests

Not sure what we were trying to do, but apparently we only use the keys
to generate the id string. How did "please" end up here?

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize 2023-07-07 09:57:49 UTC
Going straight through to PQA as these are really just minor follow-ups to squash warnings and improve a tiny piece of code style on bug 23336.
Comment 8 Tomás Cohen Arazi 2023-07-07 12:19:06 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 9 Martin Renvoize 2023-07-07 12:29:49 UTC
(In reply to Jonathan Druart from comment #3)
> I don't think it's needed but I would confirm it just in case.

I suspected that.. but if we do that I don't understand what the benefit is to just calling the find method.