Bug 20185 - Some scripts don't pass perl -wc
Summary: Some scripts don't pass perl -wc
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 20201
  Show dependency treegraph
 
Reported: 2018-02-12 16:57 UTC by Jonathan Druart
Modified: 2018-06-04 20:15 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 20185: Remove warnings from aqplan.pl (1.91 KB, patch)
2018-02-12 16:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20185: Remove warnings from stats.print.pl (1.19 KB, patch)
2018-02-12 18:39 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20185: Remove warnings from aqplan.pl (1.96 KB, patch)
2018-02-13 18:23 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 20185: Remove warnings from stats.print.pl (1.23 KB, patch)
2018-02-13 18:23 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 20185: Remove warnings from aqplan.pl (2.02 KB, patch)
2018-02-16 10:18 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 20185: Remove warnings from stats.print.pl (1.30 KB, patch)
2018-02-16 10:18 UTC, Julian Maurice
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2018-02-12 16:57:25 UTC

    
Comment 1 Jonathan Druart 2018-02-12 16:58:11 UTC
I guess it's the consequence of the push of patches related to bug 2505
Comment 2 Jonathan Druart 2018-02-12 16:58:42 UTC
Created attachment 71484 [details] [review]
Bug 20185: Remove warnings from aqplan.pl
Comment 3 Jonathan Druart 2018-02-12 18:39:30 UTC
Created attachment 71497 [details] [review]
Bug 20185: Remove warnings from stats.print.pl

This needs a full rewrite!
Comment 4 Jonathan Druart 2018-02-12 19:00:42 UTC
Sounds like there are no more.
Comment 5 M. Tompsett 2018-02-12 20:17:02 UTC
(In reply to Jonathan Druart from comment #1)
> I guess it's the consequence of the push of patches related to bug 2505

And people changing strict/warnings, or some combination thereof, into Modern::Perl, without checking? I did comment in a warning fashion on bug 20000 comment #2.
Comment 6 M. Tompsett 2018-02-12 20:22:09 UTC
Running the following to confirm comment #4:
 for i in `find . -type f -name "*.p[lm]"`; do perl -wc $i; done
Comment 7 M. Tompsett 2018-02-13 01:06:41 UTC
After getting Catmandu stuff better installed, and hacking softlinks in the path to VerboseWarnings.pm and TmplTokenizer.pm, most problems go away. These two patches are fine. However, what about the horrors of files with multiple packages: C4/Barcodes.pm and OpenILS/QueryParser.pm?
Comment 8 M. Tompsett 2018-02-13 01:07:51 UTC
Comment on attachment 71484 [details] [review]
Bug 20185: Remove warnings from aqplan.pl

Review of attachment 71484 [details] [review]:
-----------------------------------------------------------------

::: admin/aqplan.pl
@@ +36,4 @@
>  use C4::Debug;
>  use Koha::Acquisition::Currencies;
>  
> +our $input = new CGI;

If we are already tweaking the line, why not the new syntax? CGI->new?
Comment 9 Jonathan Druart 2018-02-13 14:12:17 UTC
(In reply to M. Tompsett from comment #6)
> Running the following to confirm comment #4:
>  for i in `find . -type f -name "*.p[lm]"`; do perl -wc $i; done

or simply `find . -name "*.pl" -exec perl -wc {} \;` yes.

(In reply to M. Tompsett from comment #7)
> After getting Catmandu stuff better installed, and hacking softlinks in the
> path to VerboseWarnings.pm and TmplTokenizer.pm, most problems go away.
> These two patches are fine. However, what about the horrors of files with
> multiple packages: C4/Barcodes.pm and OpenILS/QueryParser.pm?

The rewrite of these modules is for somewhere else (and somebody else :))

(In reply to M. Tompsett from comment #8)
> If we are already tweaking the line, why not the new syntax? CGI->new?

Feel free to open a bug report to replace all the occurrence of "new CGI" if you think it is useful/major.


The variable scoping problem is critical under plack.
Comment 10 M. Tompsett 2018-02-13 18:23:38 UTC
Created attachment 71561 [details] [review]
Bug 20185: Remove warnings from aqplan.pl

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 11 M. Tompsett 2018-02-13 18:23:41 UTC
Created attachment 71562 [details] [review]
Bug 20185: Remove warnings from stats.print.pl

This needs a full rewrite!

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 12 Julian Maurice 2018-02-16 09:53:58 UTC
git grep 'stats.print.pl' returns nothing and when run from a shell it dies with 
Undefined subroutine &main::getcredits called at reports/stats.print.pl line 126.

Is this script still used ?
Comment 13 Julian Maurice 2018-02-16 10:15:50 UTC
The last occurence of stats.print.pl was removed by

commit ca4b23e929b2636054263cab35f50a07dc8a02e4
Author:     Chris Cormack <crc@liblime.com>
AuthorDate: Mon Jan 7 20:42:36 2008 -0600
Commit:     Joshua Ferraro <jmf@liblime.com>
CommitDate: Mon Jan 7 20:49:16 2008 -0600

    Work in progress, working on the til reconciliation report

    More patches to come

    Signed-off-by: Joshua Ferraro <jmf@liblime.com>

I will open a new report to remove this script.
Comment 14 Julian Maurice 2018-02-16 10:18:08 UTC
Created attachment 71708 [details] [review]
Bug 20185: Remove warnings from aqplan.pl

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 15 Julian Maurice 2018-02-16 10:18:11 UTC
Created attachment 71709 [details] [review]
Bug 20185: Remove warnings from stats.print.pl

This needs a full rewrite!

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 16 Julian Maurice 2018-02-16 10:23:47 UTC
Passed QA.

Script removal in bug 20225
Comment 17 Jonathan Druart 2018-02-16 17:07:12 UTC
Pushed to master for 18.05, thanks to everybody involved!