Some of the marc21 leader plugins refer to leader.pl and include unused code. Have to check also if attached template is used at all. Pertains to: marc21_leader_book.pl marc21_leader_computerfile.pl marc21_leader_video.pl
Created attachment 40997 [details] [review] Bug 14201: Plugin marc21_leader_book is useless A closer look at this plugin revealed the following: [1] The leader_book launcher redirects to template leader.tt; so the leader_book.tt is not used at all. [2] A diff of the two templates showed that most changes are minimal and irrelevant. Some updates to leader have not been incorporated into the unused leader_book. [3] Leader_book does not use Focus while leader does. This patch removes all traces of leader_book :) It includes a tiny db revision for those brave people using it. The only behavior change for them is the focus change. Another small change: we do not need Search.pm in marc21_leader. Test plan: [1] Run the db revision. If you had some field attached to leader_book, it should be gone. [2] Check if the leader plugin still works as expected. [3] Run the FrameworkPlugin unit test. Ignore warnings from field_007. [4] Git grep on marc21_leader_book. You will only find references to PO files in misc/translator.
Created attachment 40998 [details] [review] Bug 14201: We can do without plugin marc21_leader_computerfile A closer look at this plugin revealed the same things as the previous patch for leader_book. [1] Template leader_computerfile is not used. [2] The unused template is not up-to-date either. [3] Leader_computerfile does not use Focus while leader does. This patch removes all traces of leader_computerfile. It includes a tiny db revision for those brave people using it. The only behavior change for them is the focus change. Test plan: [1] Run the db revision. [2] Check if the leader plugin still works as expected. [3] Git grep on marc21_leader_computerfile. Ignore PO files.
Created attachment 40999 [details] [review] Bug 14201: Yes, goodbye to the third plugin marc21_leader_video too A closer look at this plugin revealed the same things as earlier: [1] Since the click redirects to leader, the launcher is not used. In this case no template file existed. [2] Leader_video does not use Focus while leader does. This patch removes 'all' traces of leader_video. It includes a tiny db revision for those brave people using it. The only behavior change for them is the focus change. Test plan: [1] Run the db revision. [2] Check if the leader plugin still works as expected. [3] Git grep on marc21_leader_video. Ignore atomicupdate. No PO files :)
Created attachment 41000 [details] [review] Bug 14201: Plugin marc21_leader_book is useless A closer look at this plugin revealed the following: [1] The leader_book builder redirects to leader.pl; the launcher code with leader_book.tt is dead. [2] A diff of the two templates showed that most changes are minimal and irrelevant. Some updates to leader have not been incorporated into the unused leader_book. [3] Leader_book does not use Focus while leader does. This patch removes all traces of leader_book :) It includes a tiny db revision for those brave people using it. The only behavior change for them is the focus change. Another small change: we do not need Search.pm in marc21_leader. Test plan: [1] Run the db revision. If you had some field attached to leader_book, it should be gone. [2] Check if the leader plugin still works as expected. [3] Run the FrameworkPlugin unit test. Ignore warnings from field_007. [4] Git grep on marc21_leader_book. You will only find references to PO files in misc/translator.
Created attachment 41001 [details] [review] Bug 14201: We can do without plugin marc21_leader_computerfile A closer look at this plugin revealed the same things as the previous patch for leader_book. [1] Template leader_computerfile is not used. [2] The unused template is not up-to-date either. [3] Leader_computerfile does not use Focus while leader does. This patch removes all traces of leader_computerfile. It includes a tiny db revision for those brave people using it. The only behavior change for them is the focus change. Test plan: [1] Run the db revision. [2] Check if the leader plugin still works as expected. [3] Git grep on marc21_leader_computerfile. Ignore PO files.
Created attachment 41002 [details] [review] Bug 14201: Yes, goodbye to the third plugin marc21_leader_video too A closer look at this plugin revealed the same things as earlier: [1] Since the click redirects to leader, the launcher is not used. In this case no template file existed. [2] Leader_video does not use Focus while leader does. This patch removes 'all' traces of leader_video. It includes a tiny db revision for those brave people using it. The only behavior change for them is the focus change. Test plan: [1] Run the db revision. [2] Check if the leader plugin still works as expected. [3] Git grep on marc21_leader_video. Ignore atomicupdate. No PO files :)
Just a small change in the first commit message. accuracy++
Hi Marcel you're right, the three perl scripts/templates do nothing or do the same thing, but the magic of having too many plugins perhaps is for filling different data in the frameworks. i.e., books only uses "06 - Type of record: a - Language material" but computer files uses "m - Computer file" and videos uses "g - Projected medium" With this you save time for experimented catalogers. The data in the leader is predefined and she or he doesn't need to fill at all. Just by the fact to choose the proper framework is enough. Maybe you can do a trick in your plugin to catch the framework code and redefine the leader for the type of resource/material. Regards
Created attachment 42359 [details] [review] [SIGNED-OFF]Bug 14201: Plugin marc21_leader_book is useless A closer look at this plugin revealed the following: [1] The leader_book builder redirects to leader.pl; the launcher code with leader_book.tt is dead. [2] A diff of the two templates showed that most changes are minimal and irrelevant. Some updates to leader have not been incorporated into the unused leader_book. [3] Leader_book does not use Focus while leader does. This patch removes all traces of leader_book :) It includes a tiny db revision for those brave people using it. The only behavior change for them is the focus change. Another small change: we do not need Search.pm in marc21_leader. Test plan: [1] Run the db revision. If you had some field attached to leader_book, it should be gone. [2] Check if the leader plugin still works as expected. [3] Run the FrameworkPlugin unit test. Ignore warnings from field_007. [4] Git grep on marc21_leader_book. You will only find references to PO files in misc/translator. Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Created attachment 42360 [details] [review] [SIGNED-OFF]Bug 14201: We can do without plugin marc21_leader_computerfile A closer look at this plugin revealed the same things as the previous patch for leader_book. [1] Template leader_computerfile is not used. [2] The unused template is not up-to-date either. [3] Leader_computerfile does not use Focus while leader does. This patch removes all traces of leader_computerfile. It includes a tiny db revision for those brave people using it. The only behavior change for them is the focus change. Test plan: [1] Run the db revision. [2] Check if the leader plugin still works as expected. [3] Git grep on marc21_leader_computerfile. Ignore PO files. Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Created attachment 42361 [details] [review] [SIGNED-OFF]Bug 14201: Yes, goodbye to the third plugin marc21_leader_video too A closer look at this plugin revealed the same things as earlier: [1] Since the click redirects to leader, the launcher is not used. In this case no template file existed. [2] Leader_video does not use Focus while leader does. This patch removes 'all' traces of leader_video. It includes a tiny db revision for those brave people using it. The only behavior change for them is the focus change. Test plan: [1] Run the db revision. [2] Check if the leader plugin still works as expected. [3] Git grep on marc21_leader_video. Ignore atomicupdate. No PO files :) Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
(In reply to Héctor Eduardo Castro Avalos from comment #9) > Hi Marcel you're right, the three perl scripts/templates do nothing or do > the same thing, but the magic of having too many plugins perhaps is for > filling different data in the frameworks. i.e., books only uses "06 - Type > of record: a - Language material" but computer files uses "m - Computer > file" and videos uses "g - Projected medium" > > With this you save time for experimented catalogers. The data in the leader > is predefined and she or he doesn't need to fill at all. Just by the fact to > choose the proper framework is enough. > > Maybe you can do a trick in your plugin to catch the framework code and > redefine the leader for the type of resource/material. Yes, you are right that this would be handy. And it probably was the idea when these files came up. But it did not work. Note that this report does not change behavior. So I am opening a new report to collect some ideas how to improve the default leader. Since we have the framework code already, we could pass that one to the plugin. When we add a preference, we could add leaders there per framework code. I would not favor hardcoding BKS BOOKS etc. in the plugin.. See bug 14815.
Are we sure they are useless? Maybe there is a bug (the plugin_name parameter could be wrong) in them. The differences are, for marc21_leader_computerfile vs marc21_leader: my $f2023 = substr( $result, 20, 4 ); "f2023" => $f2023, These 2 lines exist in marc21_leader, no in marc21_leader_computerfile. Could we ask for a functional point of view, to know if we should remove them or fix them?
(In reply to Jonathan Druart from comment #14) > Are we sure they are useless? > Maybe there is a bug (the plugin_name parameter could be wrong) in them. > > The differences are, for marc21_leader_computerfile vs marc21_leader: > my $f2023 = substr( $result, 20, 4 ); > "f2023" => $f2023, > These 2 lines exist in marc21_leader, no in marc21_leader_computerfile. > > Could we ask for a functional point of view, to know if we should remove > them or fix them? See also my commit messages. I checked it too. The computerfile is out of date. The difference above is not really interesting. The positions 20-23 are hardcoded 4500 (see template too) and should not be changed at all.
btw the leader template refers to pos 20-24 in the text this should be 20-23 (see loc)
Created attachment 42675 [details] [review] Bug 14201: Plugin marc21_leader_book is useless A closer look at this plugin revealed the following: [1] The leader_book builder redirects to leader.pl; the launcher code with leader_book.tt is dead. [2] A diff of the two templates showed that most changes are minimal and irrelevant. Some updates to leader have not been incorporated into the unused leader_book. [3] Leader_book does not use Focus while leader does. This patch removes all traces of leader_book :) It includes a tiny db revision for those brave people using it. The only behavior change for them is the focus change. Another small change: we do not need Search.pm in marc21_leader. Test plan: [1] Run the db revision. If you had some field attached to leader_book, it should be gone. [2] Check if the leader plugin still works as expected. [3] Run the FrameworkPlugin unit test. Ignore warnings from field_007. [4] Git grep on marc21_leader_book. You will only find references to PO files in misc/translator. Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 42676 [details] [review] Bug 14201: We can do without plugin marc21_leader_computerfile A closer look at this plugin revealed the same things as the previous patch for leader_book. [1] Template leader_computerfile is not used. [2] The unused template is not up-to-date either. [3] Leader_computerfile does not use Focus while leader does. This patch removes all traces of leader_computerfile. It includes a tiny db revision for those brave people using it. The only behavior change for them is the focus change. Test plan: [1] Run the db revision. [2] Check if the leader plugin still works as expected. [3] Git grep on marc21_leader_computerfile. Ignore PO files. Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 42677 [details] [review] Bug 14201: Yes, goodbye to the third plugin marc21_leader_video too A closer look at this plugin revealed the same things as earlier: [1] Since the click redirects to leader, the launcher is not used. In this case no template file existed. [2] Leader_video does not use Focus while leader does. This patch removes 'all' traces of leader_video. It includes a tiny db revision for those brave people using it. The only behavior change for them is the focus change. Test plan: [1] Run the db revision. [2] Check if the leader plugin still works as expected. [3] Git grep on marc21_leader_video. Ignore atomicupdate. No PO files :) Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Patch pushed to master. Thanks Marcel!