https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690
--- Comment #22 from Joonas Kylmälä <[email protected]> --- (In reply to Martin Renvoize from comment #21) > Comment on attachment 98143 [details] [review] > Bug 22690: Refactor merging of records to improve performance (Elasticsearch) > > Review of attachment 98143 [details] [review]: > ----------------------------------------------------------------- > > This looks like a reasonable approach to me and seems to work well.. a > relatively minor point regarding the introduce Koha/Biblio method. > > ::: Koha/Biblio.pm > @@ +854,5 @@ > > +Move items from the given biblio > > + > > +=cut > > + > > +sub move_items_from_biblio { > > I feel like 'move_items_from_biblio' isn't immediately obvious as a function > name.. are we moving from 'this' biblio or to it.. perhaps > 'adopt_items_from_biblio' is a bit clearer? I would just reverse the original idea: move_items_to_biblio. Then it should be obvious we are talking about the current object's items being moved. Not sure if perl OO style supports overloading, but in that case it could be just move_items, and the parameters defined whether we move them to another biblio or (yet to be included to Koha) to a holdings record. -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
