http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13223 --- Comment #1 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 33381 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33381&action=edit Bug 13223: Plugin housekeeping: do not redefine wrapper This report is connected to bug 10480 which will change the general mechanism of loading plugins, but can be tested independently and ahead of that proposed change. Several unimarc plugins use a wrapper sub. The code of this subroutine is not exactly the same for all plugins: in some cases the routine is extended for double character strings (dblspace and dblpipe). It would not hurt to use the extended code for all plugins. By moving the code to a module in the local dir, we prevent redefinition when two or more plugins are loading wrapper in a do-statement. NOTE: You will not see wrapper redefine errors in your log, since the plugins do not use the warnings pragma (yet). Since this patch touches seventeen unimarc plugins, a unimarc signoff is preferred :) Test plan: Use some plugins changed in this patch (if not in use already). Load the MARC editor. Click on some tag editor-buttons to check unchanged behavior. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13223 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |frederic@tamil.fr --- Comment #2 from Frédéric Demians <frederic@tamil.fr> --- Good initiative. Shouldn't you put your module in C4 (C4::Utils) directory, rather than in plugin directory itself? -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13223 --- Comment #4 from Frédéric Demians <frederic@tamil.fr> ---

I thought of that, but its use seems to be so connected to these plugins that I did not move it away from them. Do you prefer to have them there?

Yes. I understand your reasoning. Except in misc/translator, I don't see any directory other than 'Koha' or 'C4' containing module files. We can imagine scenario in the future where Koha libraries could be searched differently than with PERL5LIB environment variable. Another point: You module need a copyright, I suppose. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13223 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33484|0 |1 is obsolete| | --- Comment #6 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 33489 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33489&action=edit Bug 13223: Plugin housekeeping: do not redefine wrapper This report is connected to bug 10480 which will change the general mechanism of loading plugins, but can be tested independently and ahead of that proposed change. Several unimarc plugins use a wrapper sub. The code of this subroutine is not exactly the same for all plugins: in some cases the routine is extended for double character strings (dblspace and dblpipe). It would not hurt to use the extended code for all plugins. By moving the code to a module, we prevent redefinition when two or more plugins are loading wrapper in a do-statement. NOTE: You will not see wrapper redefine errors in your log, since the plugins do not use the warnings pragma (yet). Since this patch touches seventeen unimarc plugins, a unimarc signoff is preferred :) Test plan: Use some plugins changed in this patch (if not in use already). Load the MARC editor. Click on some tag editor-buttons to check unchanged behavior. Signed-off-by: Frederic Demians <f.demians@tamil.fr> Unimarc plugins work as usual. No regression. Simple code factorization. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13223 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13223 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33489|0 |1 is obsolete| | Attachment #33493|0 |1 is obsolete| | --- Comment #9 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 33536 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33536&action=edit [PASSED QA] Bug 13223: Plugin housekeeping: do not redefine wrapper This report is connected to bug 10480 which will change the general mechanism of loading plugins, but can be tested independently and ahead of that proposed change. Several unimarc plugins use a wrapper sub. The code of this subroutine is not exactly the same for all plugins: in some cases the routine is extended for double character strings (dblspace and dblpipe). It would not hurt to use the extended code for all plugins. By moving the code to a module, we prevent redefinition when two or more plugins are loading wrapper in a do-statement. NOTE: You will not see wrapper redefine errors in your log, since the plugins do not use the warnings pragma (yet). Since this patch touches seventeen unimarc plugins, a unimarc signoff is preferred :) Test plan: Use some plugins changed in this patch (if not in use already). Load the MARC editor. Click on some tag editor-buttons to check unchanged behavior. Signed-off-by: Frederic Demians <f.demians@tamil.fr> Unimarc plugins work as usual. No regression. Simple code factorization. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13223 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #11 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I know that the new module is quite simple, but please add some unit tests :) -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13223 --- Comment #13 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 33542 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33542&action=edit Bug 13223: Follow-up - Adding some unit tests -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13223 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33542|0 |1 is obsolete| | --- Comment #15 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 33546 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33546&action=edit Bug 13223: Follow-up - Adding some unit tests Signed-off-by: Frederic Demians <f.demians@tamil.fr> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13223 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Comment #17 is|1 |0 private| | -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13223 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@biblibre.co | |m --- Comment #18 from Jonathan Druart <jonathan.druart@biblibre.com> --- We must be careful that Koha::Util does not get messy. IMO we should not use it at all, that force developers to use something else. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13223 --- Comment #20 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #18)

We must be careful that Koha::Util does not get messy. IMO we should not use it at all, that force developers to use something else.

I agree on the first line. But the four line wrapper thing is a typical example of a util. (Note that I initially left it in cataloguing/value_builder, but was asked to move it to a util folder.) -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13223 --- Comment #22 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #21)

(In reply to Katrin Fischer from comment #19)

...

Jonathan - where would you put it?

I don't know :) Koha::Cataloguing::Plugin?

No please, bug 10480 will add a real object for plugins. Work in progress..

At some points, all can be considered as a "util" :) It was just a quick note, I did not want to block this patch. Great.

-- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13223 --- Comment #24 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Katrin Fischer from comment #23)

Bit worried about adding 'plugin' confusion, as we also have the younger plugin system and TT plugins...

Thats why I renamed to FrameworkPlugin. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13223 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|trivial |normal -- You are receiving this mail because: You are watching all bug changes.