Bug 16203 - Convert item plugins to new style (see bug 10480)
Summary: Convert item plugins to new style (see bug 10480)
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 19595
  Show dependency treegraph
 
Reported: 2016-04-05 12:48 UTC by Marcel de Rooy
Modified: 2018-06-04 20:10 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 16203: item plugins (W-I-P) (15.70 KB, patch)
2016-04-05 14:15 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16203: Convert item plugins to new style (see bug 10480) (24.49 KB, patch)
2016-04-08 10:32 UTC, Marcel de Rooy
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 16203: Convert item plugins to new style (see bug 10480) (24.51 KB, patch)
2016-12-05 12:42 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 16203: Convert item plugins to new style (see bug 10480) (24.57 KB, patch)
2016-12-14 17:43 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2016-04-05 12:48:53 UTC

    
Comment 1 Marcel de Rooy 2016-04-05 14:15:28 UTC Comment hidden (obsolete)
Comment 2 Marcel de Rooy 2016-04-08 10:32:50 UTC
Created attachment 50066 [details] [review]
Bug 16203: Convert item plugins to new style (see bug 10480)

Converts item plugins to new style (with builder and launcher).
See also bugs 10480 and 13437.

The following plugins have been adjusted:
barcode_manual.pl
barcode.pl
callnumber-KU.pl
callnumber.pl
cn_browser.pl (Added license statement too)
dateaccessioned.pl
macles.pl
stocknumberam123.pl
stocknumberAV.pl
stocknumber.pl

Test plan:
Connect the plugin to an item field.
Verify that the plugin still works.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested if all plugins compile okay.
Ran most of them thru FrameworkPlugin.t.
Tested them in the item editor.

Note: the form for macles.pl comes up, further hard to test.
Comment 3 Josef Moravec 2016-12-02 14:39:50 UTC
Looks good, all plugins are working but QA tool says:

 FAIL   cataloguing/value_builder/barcode.pl
   FAIL   valid
                "my" variable $DEBUG masks earlier declaration in same scope

 FAIL   cataloguing/value_builder/barcode_manual.pl
   FAIL   valid
                "my" variable $DEBUG masks earlier declaration in same scope
Comment 4 Marcel de Rooy 2016-12-05 12:24:47 UTC
Thanks, Josef, for testing.
You can safely ignore these warnings. They are not coming from the changes in this patch set, but from a (unmodified) module used by these plugins.

This is the source:
$ perl -cw C4/Barcodes/ValueBuilder.pm
"my" variable $DEBUG masks earlier declaration in same scope at C4/Barcodes/ValueBuilder.pm line 43.
"my" variable $DEBUG masks earlier declaration in same scope at C4/Barcodes/ValueBuilder.pm line 85.
C4/Barcodes/ValueBuilder.pm syntax OK

If you look at that module, you will see that it actually contains multiple packages and each one having a $DEBUG variable.
A construction like that is not recommended and fortunately an exception in Koha, but it should work. The warning is actually dubious.

All ready for a signoff ? :)
Comment 5 Josef Moravec 2016-12-05 12:41:15 UTC
(In reply to Marcel de Rooy from comment #4)
> 
> All ready for a signoff ? :)

I think so ;)
Comment 6 Josef Moravec 2016-12-05 12:42:02 UTC
Created attachment 57956 [details] [review]
[SIGNED-OFF] Bug 16203: Convert item plugins to new style (see bug 10480)

Converts item plugins to new style (with builder and launcher).
See also bugs 10480 and 13437.

The following plugins have been adjusted:
barcode_manual.pl
barcode.pl
callnumber-KU.pl
callnumber.pl
cn_browser.pl (Added license statement too)
dateaccessioned.pl
macles.pl
stocknumberam123.pl
stocknumberAV.pl
stocknumber.pl

Test plan:
Connect the plugin to an item field.
Verify that the plugin still works.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested if all plugins compile okay.
Ran most of them thru FrameworkPlugin.t.
Tested them in the item editor.

Note: the form for macles.pl comes up, further hard to test.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 7 Marcel de Rooy 2016-12-05 12:46:23 UTC
(In reply to Josef Moravec from comment #6)
> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Great! Thx.
Comment 8 Jonathan Druart 2016-12-14 12:49:45 UTC
 FAIL   cataloguing/value_builder/barcode.pl
   FAIL   valid
                "my" variable $DEBUG masks earlier declaration in same scope 

 FAIL   cataloguing/value_builder/barcode_manual.pl
   FAIL   valid
                "my" variable $DEBUG masks earlier declaration in same scope
Comment 9 Marcel de Rooy 2016-12-14 12:55:15 UTC
(In reply to Jonathan Druart from comment #8)
>  FAIL   cataloguing/value_builder/barcode.pl
>    FAIL   valid
>                 "my" variable $DEBUG masks earlier declaration in same scope 
> 
>  FAIL   cataloguing/value_builder/barcode_manual.pl
>    FAIL   valid
>                 "my" variable $DEBUG masks earlier declaration in same scope

Please read comment4.
Back to SO
Comment 10 Jonathan Druart 2016-12-14 17:43:01 UTC
Created attachment 58185 [details] [review]
Bug 16203: Convert item plugins to new style (see bug 10480)

Converts item plugins to new style (with builder and launcher).
See also bugs 10480 and 13437.

The following plugins have been adjusted:
barcode_manual.pl
barcode.pl
callnumber-KU.pl
callnumber.pl
cn_browser.pl (Added license statement too)
dateaccessioned.pl
macles.pl
stocknumberam123.pl
stocknumberAV.pl
stocknumber.pl

Test plan:
Connect the plugin to an item field.
Verify that the plugin still works.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested if all plugins compile okay.
Ran most of them thru FrameworkPlugin.t.
Tested them in the item editor.

Note: the form for macles.pl comes up, further hard to test.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 11 Jonathan Druart 2016-12-14 17:43:52 UTC
(In reply to Marcel de Rooy from comment #9)
> (In reply to Jonathan Druart from comment #8)
> >  FAIL   cataloguing/value_builder/barcode.pl
> >    FAIL   valid
> >                 "my" variable $DEBUG masks earlier declaration in same scope 
> > 
> >  FAIL   cataloguing/value_builder/barcode_manual.pl
> >    FAIL   valid
> >                 "my" variable $DEBUG masks earlier declaration in same scope
> 
> Please read comment4.
> Back to SO

Sorry about that.
Yes it's because we now "use" intead of "require".
These DEBUG variables are completely useless and could be removed, but it's outside the scope of this bug.
Comment 12 Kyle M Hall (khall) 2016-12-16 11:55:22 UTC
Pushed to master for 17.05, thanks Marcel!
Comment 13 Katrin Fischer 2016-12-18 20:38:08 UTC
This won't get backported to 16.11.x as it is an enhancement.