[Bug 29859] New: Favor iterators over as_list
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 Bug ID: 29859 Summary: Favor iterators over as_list Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: ASSIGNED Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: jonathan.druart+koha@gmail.com Reporter: jonathan.druart+koha@gmail.com QA Contact: testopia@bugs.koha-community.org Depends on: 29844 On bug 29844 we decided to remove wantarray from Koha::Objects->search. Reviewing the difference occurrences I found some unnecessary uses of ->as_list, where iterators should be used instead. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29844 [Bug 29844] Remove uses of wantarray in Koha::Objects -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 --- Comment #1 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 129346 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129346&action=edit Bug 29859: Use iterator instead of as_list On bug 29844 we decided to remove wantarray from Koha::Objects->search. Reviewing the difference occurrences I found some unnecessary uses of ->as_list, where iterators should be used instead. This patch only removes the obvious places, not the tricky ones. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 --- Comment #2 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- There is a non-squashed version on https://gitlab.com/joubu/Koha/-/tree/bug_29859 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #129346|0 |1 is obsolete| | --- Comment #3 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 129350 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129350&action=edit Bug 29859: Use iterator instead of as_list On bug 29844 we decided to remove wantarray from Koha::Objects->search. Reviewing the difference occurrences I found some unnecessary uses of ->as_list, where iterators should be used instead. This patch only removes the obvious places, not the tricky ones. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |tomascohen@gmail.com |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|tomascohen@gmail.com |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Medium patch Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #129350|0 |1 is obsolete| | --- Comment #4 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 130102 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130102&action=edit Bug 29859: Use iterator instead of as_list On bug 29844 we decided to remove wantarray from Koha::Objects->search. Reviewing the difference occurrences I found some unnecessary uses of ->as_list, where iterators should be used instead. This patch only removes the obvious places, not the tricky ones. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Comment on attachment 130102 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130102 Bug 29859: Use iterator instead of as_list Review of attachment 130102: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=29859&attachment=130102) ----------------------------------------------------------------- We introduce a few .count calls in templates here.. which will add a DB hit per call unless I'm mistaken? Now, I don't think a count call has much impact on performance, but it may be worth taking into consideration at some point.. for the pattern [% IF whatevers.count %]boiler[% FOREACH whatever IN whatever %]list of stuff[% END %][% END %] we could perhaps just add the boiler in the first iteration of the loop using the 'first' or 'index' methods of the TT iterator? instead of testing for count explicitly? ::: Koha/Holds.pm @@ +122,5 @@
+ { + columns => ['itemnumber'], + collapse => 1, + } + )->get_columns('itemnumber');
get_column vs get_columns -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com QA Contact|kyle@bywatersolutions.com |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=30017 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #130102|0 |1 is obsolete| | --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 130148 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130148&action=edit Bug 29859: Use iterator instead of as_list On bug 29844 we decided to remove wantarray from Koha::Objects->search. Reviewing the difference occurrences I found some unnecessary uses of ->as_list, where iterators should be used instead. This patch only removes the obvious places, not the tricky ones. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 130149 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130149&action=edit Bug 29859: (QA follow-up) Minor typo correction We introduced a tiny bug with a typo in the patchset, the accessor method is called 'get_column' not 'get_columns'. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #8 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- A nice improvement.. I fixed the minor typo with a followup and moved the discussion around .count vs loop variables to a new bug. QA scripts are happy and I couldn't find any errors in the UI. Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |22.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 --- Comment #9 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to master for 22.05, thanks to everybody involved 🦄 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 --- Comment #10 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 130480 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130480&action=edit Bug 29859: (follow-up) Use iterator instead of as_list -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |additional_work_needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 --- Comment #11 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 130483 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130483&action=edit Bug 29859: Fix currencies display Oops! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 Bug 29859 depends on bug 29844, which changed state. Bug 29844 Summary: Remove uses of wantarray in Koha::Objects https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29844 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com Resolution|--- |FIXED Status|Pushed to master |RESOLVED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #12 from Fridolin Somers <fridolin.somers@biblibre.com> ---
Bug 29859: (follow-up) Use iterator instead of as_list Bug 29859: Fix currencies display Pushed to master
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|additional_work_needed | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 --- Comment #13 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 132972 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=132972&action=edit Bug 29859: Fix club/enroll -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |additional_work_needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|additional_work_needed | --- Comment #14 from Fridolin Somers <fridolin.somers@biblibre.com> ---
Bug 29859: Fix club/enroll Pushed to master
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 --- Comment #15 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 132983 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=132983&action=edit Bug 29859: Fix BatchUpdateAuthority Use of inherited AUTOLOAD for non-method Koha::BackgroundJob::BatchUpdateAuthority::ModAuthority() is no longer allowed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #132983|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #132983|1 |0 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 --- Comment #16 from Fridolin Somers <fridolin.somers@biblibre.com> ---
Bug 29859: Fix BatchUpdateAuthority Pushed to master
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 --- Comment #17 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Fridolin Somers from comment #16)
Bug 29859: Fix BatchUpdateAuthority Pushed to master
I obsoleted this.(In reply to Fridolin Somers from comment #16)
Bug 29859: Fix BatchUpdateAuthority Pushed to master
Hum? This patch was marked as obsolete, it shouldn't have been pushed. It was a mistake to attach it here. See bug 30461. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #132983|0 |1 is obsolete| | --- Comment #18 from Fridolin Somers <fridolin.somers@biblibre.com> --- Comment on attachment 132983 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=132983 Bug 29859: Fix BatchUpdateAuthority Ah ok, reverted from master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29859 Lari Taskula <lari.taskula@hypernova.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |39265 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39265 [Bug 39265] Self registration: Hide library from the form if there is only one library -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org