Bug 16203

Summary: Convert item plugins to new style (see bug 10480)
Product: Koha Reporter: Marcel de Rooy <m.de.rooy>
Component: CatalogingAssignee: Marcel de Rooy <m.de.rooy>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: jonathan.druart, josef.moravec, katrin.fischer, kyle, m.de.rooy
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 19595    
Attachments: Bug 16203: item plugins (W-I-P)
Bug 16203: Convert item plugins to new style (see bug 10480)
[SIGNED-OFF] Bug 16203: Convert item plugins to new style (see bug 10480)
Bug 16203: Convert item plugins to new style (see bug 10480)

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 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.