https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22531
Nick Clemens <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |[email protected] --- Comment #33 from Nick Clemens <[email protected]> --- Reading through the code here, my best guess is that many decisions were made in the interest of preserving functionality of existing backends, which I think can be fine, but a few things I would like to see comments, maybe FIXMEs for: 1 - Accepting either a hash or a scalar seems a bit odd, at leats an explanation would help - probably a FIXME 2 - Retrieving ILL partner patrons using a string of emails seems possibly dangerous. I imagine past logs contain only these. If going forward we could use borrowernumbers or something else, that would be ideal. If that would break existing backends an explanation would be nice 3 - Some function names and variables could be made clearer, i.e.: [% FOREACH add IN log.info.additional.partner_email_previous.split('; ') %] add could be 'additional_borrower' Koha::Illrequest::Logger->logged_requested_partners($log); maybe 'return_requested_partners'? the current seems unclear I think with some clarity and cleanup this one can move forward -- 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/
