Notice then numerous my variables at module level. There is also a call to C4::Context->preference in a BEGIN block, which is bad.
Created attachment 32092 [details] [review] Bug 10126: Remove my variables at module level In C4::Reports::Guided, a lot of variables was defined at module level and reused in subroutine. I didn't find any problem with Plack, so I'm not sure this patch is useful. Test plan: Verify there is no more ^my in the module and you don't find any regression with the guided reports tools (with and without Plack)
This patch is quite ugly, feel free to provide your own version ;)
Created attachment 32100 [details] [review] Bug 10126: Remove my variables at module level In C4::Reports::Guided, a lot of variables was defined at module level and reused in subroutine. I didn't find any problem with Plack, so I'm not sure this patch is useful. Test plan: Verify there is no more ^my in the module and you don't find any regression with the guided reports tools (with and without Plack) Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> I'm unsure if this is needed, but I have verified it causes no regressions
QA comments : * passes koha-qa.pl * guided reports also still work in CGI mode * no other comments, looks good passed QA
Created attachment 32199 [details] [review] Bug 10126: Remove my variables at module level In C4::Reports::Guided, a lot of variables was defined at module level and reused in subroutine. I didn't find any problem with Plack, so I'm not sure this patch is useful. Test plan: Verify there is no more ^my in the module and you don't find any regression with the guided reports tools (with and without Plack) Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> I'm unsure if this is needed, but I have verified it causes no regressions Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
Patch pushed to master. Thanks Jonathan.
Created attachment 32409 [details] [review] Bug 10126: (qa followup) fix tests It seems the removal of global variables changes the behaviour of Test::MockModule, and it now expects the namespace in front of the statically called method. Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>