Because adding parameters to pass to called subroutines is difficult, change parameters to be a hashref.
Created attachment 65531 [details] [review] Bug 19040: Refactor GetMarcBiblio parameters Change parameters to a hashref. Test plan to come later.
Created attachment 65695 [details] [review] Bug 19040: print STDERRs to develop test plan
TEST PLAN --------- perldoc C4::Biblio prove t/Biblio.t prove t/db_dependent/Acquisition/OrderFromSubscription.t prove t/db_dependent/AdditionalField.t prove t/db_dependent/Bookseller.t prove t/db_dependent/Letters.t prove t/db_dependent/Serials/Claims.t prove t/db_dependent/Serials_2.t prove t/db_dependent/Authorities/Merge.t prove t/db_dependent/Biblio.t prove t/db_dependent/Exporter/Record.t prove t/db_dependent/Record/marcrecord2csv.t prove t/db_dependent/HoldsQueue.t prove t/db_dependent/Items.t prove t/db_dependent/Koha/Filter/EmbedItemsAvailability.t prove t/db_dependent/OAI/Sets.t prove t/db_dependent/Koha/BiblioUtils.t prove t/db_dependent/OAI/Server.t prove t/db_dependent/Reserves/MultiplePerRecord.t prove t/db_dependent/ShelfBrowser.t run the koha qa test tools -- if the changes have a typo, they'll fail. eyeball each change: - have the parameters changed? That is, should biblionumber, embeditems, and opac be the same expected values as before?
Comment on attachment 65695 [details] [review] Bug 19040: print STDERRs to develop test plan Not needed to push.
Created attachment 66101 [details] [review] Bug 19040: Refactor GetMarcBiblio parameters Change parameters to a hashref. Rebased: 2017-08-16
Created attachment 66145 [details] [review] [SIGNED-OFF] Bug 19040: Refactor GetMarcBiblio parameters Change parameters to a hashref. Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 66457 [details] [review] Bug 19040: Refactor GetMarcBiblio parameters Change parameters to a hashref. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Looks good to me. Two calls in migration_tools/22_to_30 still in old style.
Created attachment 66458 [details] [review] Bug 19040: [QA Follow-up] Adjust embed_items parameter in showdiffmarc Trivial adjustment Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
carp 'GetMarcBiblio called without parameters'; Should be a Koha::Exception. See coding guidelines. git grep -E "GetMarcBiblio\s*\(\s*\$" No results as expected git grep -E "GetMarcBiblio\s*\(\s*[^{]" misc/migration_tools/22_to_30/missing090field.pl: my $record = GetMarcBiblio($biblionumber); misc/migration_tools/22_to_30/rebuild_unimarc_100.pl: my $record = GetMarcBiblio($biblionumber); [OK] t/Biblio.t:warning_is { $ret = GetMarcBiblio() } Who is going to run these scripts still? And from what version? Should we still have in master ??
(In reply to Marcel de Rooy from comment #9) > Who is going to run these scripts still? And from what version? Should we > still have in master ?? People upgrading from Koha 2.2?
Created attachment 66477 [details] [review] Bug 19040: Update 2 occurrences of GetMarcBiblio in 22_to_30
Pushed to master for 17.11, thanks to everybody involved!
(In reply to Jonathan Druart from comment #10) > (In reply to Marcel de Rooy from comment #9) > > Who is going to run these scripts still? And from what version? Should we > > still have in master ?? > > People upgrading from Koha 2.2? People upgrading from 2.2 to current are likely to stop at 3.4 before proceeding to master due to items being stored differently, at least that was my impression. That's why I didn't change those two scripts. But I suppose it can't hurt.
(In reply to Jonathan Druart from comment #10) > (In reply to Marcel de Rooy from comment #9) > > Who is going to run these scripts still? And from what version? Should we > > still have in master ?? > > People upgrading from Koha 2.2? Sure? :) My impression was that you would call 22_to_30 from 3.0. So we should not be needing them in master?
Then let's remove them :)
Enhancement not pushed to 17.05.x