Bug 28211 - Replace use of call_recursive() with call()
Summary: Replace use of call_recursive() with call()
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Plugin architecture (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on: 26352
Blocks:
  Show dependency treegraph
 
Reported: 2021-04-23 15:53 UTC by Kyle M Hall
Modified: 2021-06-29 14:14 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 28211: Replace use of call_recursive() with call() (11.99 KB, patch)
2021-04-23 15:59 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 28211 Fix count of plugins that should be installed for unit test (994 bytes, patch)
2021-05-12 13:42 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 28211: Replace use of call_recursive() with call() (12.06 KB, patch)
2021-05-12 13:53 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 28211 Fix count of plugins that should be installed for unit test (1.02 KB, patch)
2021-05-12 13:53 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 28211: Replace use of call_recursive() with call() (12.05 KB, patch)
2021-06-25 12:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 28211: Replace use of call_recursive() with call() (12.05 KB, patch)
2021-06-25 12:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 28211: Fix count of plugins that should be installed for unit test (1.04 KB, patch)
2021-06-25 12:01 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2021-04-23 15:53:30 UTC
This is based on Julian's idea on bug 28026 where we could get rid of call_recursive by passing refs as arguments to call.
Comment 1 Kyle M Hall 2021-04-23 15:59:40 UTC
Created attachment 120120 [details] [review]
Bug 28211: Replace use of call_recursive() with call()

This is based on Julian's idea on bug 28026 where we could get rid of call_recursive by passing refs as arguments to call.

Test Plan:
1) Apply this patch
2) prove t/db_dependent/Koha/Plugins/Plugins.t
3) prove t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t
Comment 2 Andrew Fuerste-Henry 2021-05-12 12:14:50 UTC
I get this test failure:
root@kohadevbox:koha(master)$ prove t/db_dependent/Koha/Plugins/Plugins.t
t/db_dependent/Koha/Plugins/Plugins.t .. 5/55 Plugin error (Test Plugin): item_barcode_transform called with parameter: 1 at /kohadevbox/koha/Koha/Plugins.pm line 74.
Plugin error (Test Plugin): item_barcode_transform called with parameter: abcd at /kohadevbox/koha/Koha/Plugins.pm line 74.

    #   Failed test 'Only four plugins found via a metadata tag'
    #   at t/db_dependent/Koha/Plugins/Plugins.t line 129.
    #          got: '2'
    #     expected: '4'
    # Looks like you failed 1 test of 2.

#   Failed test 'GetPlugins() tests'
#   at t/db_dependent/Koha/Plugins/Plugins.t line 132.
Use of uninitialized value $interface in string eq at /kohadevbox/koha/C4/Templates.pm line 279.
Use of uninitialized value $interface in string eq at /kohadevbox/koha/C4/Templates.pm line 280.
Use of uninitialized value $interface in string eq at /kohadevbox/koha/C4/Templates.pm line 282.
Use of uninitialized value $lang in concatenation (.) or string at /kohadevbox/koha/C4/Templates.pm line 296.
Use of uninitialized value $lang in concatenation (.) or string at /kohadevbox/koha/C4/Templates.pm line 296.
Use of uninitialized value $lang in concatenation (.) or string at /kohadevbox/koha/C4/Templates.pm line 296.
t/db_dependent/Koha/Plugins/Plugins.t .. 50/55 # Looks like you failed 1 test of 55.
t/db_dependent/Koha/Plugins/Plugins.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/55 subtests 

Test Summary Report
-------------------
t/db_dependent/Koha/Plugins/Plugins.t (Wstat: 256 Tests: 55 Failed: 1)
  Failed test:  7
  Non-zero exit status: 1
Files=1, Tests=55,  5 wallclock secs ( 0.02 usr  0.01 sys +  1.80 cusr  0.39 csys =  2.22 CPU)
Result: FAIL
Comment 3 Kyle M Hall 2021-05-12 13:42:22 UTC
Created attachment 120875 [details] [review]
Bug 28211 Fix count of plugins that should be installed for unit test
Comment 4 Andrew Fuerste-Henry 2021-05-12 13:53:50 UTC
Created attachment 120882 [details] [review]
Bug 28211: Replace use of call_recursive() with call()

This is based on Julian's idea on bug 28026 where we could get rid of call_recursive by passing refs as arguments to call.

Test Plan:
1) Apply this patch
2) prove t/db_dependent/Koha/Plugins/Plugins.t
3) prove t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 5 Andrew Fuerste-Henry 2021-05-12 13:53:54 UTC
Created attachment 120883 [details] [review]
Bug 28211 Fix count of plugins that should be installed for unit test

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 6 Marcel de Rooy 2021-06-23 14:13:33 UTC
I would recommend to merge this report with the call_recursive report. Both are signed off. We could eliminate the code we actually do not want to push.

The call_recursive name was a bit misleading too btw.
Comment 7 Kyle M Hall 2021-06-23 15:30:16 UTC
(In reply to Marcel de Rooy from comment #6)
> I would recommend to merge this report with the call_recursive report. Both
> are signed off. We could eliminate the code we actually do not want to push.
> 
> The call_recursive name was a bit misleading too btw.

The two bugs are on the other sides of 26351 and 26352. Merging them would require modifying the patches for those bugs, both of which have passed qa. I think we should simply push the four bugs as a unit rather than risk introducing regressions.
Comment 8 Marcel de Rooy 2021-06-24 06:02:59 UTC
(In reply to Kyle M Hall from comment #7)
> (In reply to Marcel de Rooy from comment #6)
> > I would recommend to merge this report with the call_recursive report. Both
> > are signed off. We could eliminate the code we actually do not want to push.
> > 
> > The call_recursive name was a bit misleading too btw.
> 
> The two bugs are on the other sides of 26351 and 26352. Merging them would
> require modifying the patches for those bugs, both of which have passed qa.
> I think we should simply push the four bugs as a unit rather than risk
> introducing regressions.

Okay. I missed the other two ;)
Comment 9 Marcel de Rooy 2021-06-25 08:55:10 UTC
Applying: Bug 26351: Clean up includes in Circulation.pm
error: sha1 information is lacking or useless (C4/Circulation.pm).
error: could not build fake ancestor
Patch failed at 0001 Bug 26351: Clean up includes in Circulation.pm
hint: Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-26351-Clean-up-includes-in-Circulationpm-DpTlZT.patch

Cannot apply cleanly patches from bug 26351. Everything will be left dirty.
git bz apply --continue will not continue the process if patches from other bug reports need to be applied.
Comment 10 Kyle M Hall 2021-06-25 11:59:12 UTC
Patches applied fine for me. Did you apply on top of 26352?
Comment 11 Kyle M Hall 2021-06-25 12:00:10 UTC
Created attachment 122425 [details] [review]
Bug 28211: Replace use of call_recursive() with call()

This is based on Julian's idea on bug 28026 where we could get rid of call_recursive by passing refs as arguments to call.

Test Plan:
1) Apply this patch
2) prove t/db_dependent/Koha/Plugins/Plugins.t
3) prove t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 12 Kyle M Hall 2021-06-25 12:01:02 UTC
Created attachment 122426 [details] [review]
Bug 28211: Replace use of call_recursive() with call()

This is based on Julian's idea on bug 28026 where we could get rid of call_recursive by passing refs as arguments to call.

Test Plan:
1) Apply this patch
2) prove t/db_dependent/Koha/Plugins/Plugins.t
3) prove t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 13 Kyle M Hall 2021-06-25 12:01:44 UTC
Created attachment 122427 [details] [review]
Bug 28211: Fix count of plugins that should be installed for unit test

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>