https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21395 --- Comment #1 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 79239 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79239&action=edit Bug 21395: Make perlcritic happy This patch adds a .perlcriticrc (copied from qa-test-tools) and fixes almost all perlcrictic violations according to this .perlcriticrc The remaining violations are silenced out by appending a '## no critic' to the offending lines. They can still be seen by using the --force option of perlcritic This patch also modify t/00-testcritic.t to check all Perl files using the new .perlcriticrc. I'm not sure if this test script is still useful as it is now equivalent to `perlcritic --quiet .` and it looks like it is much slower (approximatively 5 times slower on my machine) Test plan: 1. Run `perlcritic --quiet .` from the root directory. It should output nothing 2. Run `perlcritic --quiet --force .`. It should output 6 errors (5 StringyEval, 1 BarewordFileHandles) 3. Run `TEST_QA=1 prove t/00-testcritic.t` 4. Read the patch. Check that all changes make sense and do not introduce undesired behaviour -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21395 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |josef.moravec@gmail.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21395 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #79239|0 |1 is obsolete| | --- Comment #2 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 103666 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103666&action=edit Bug 21395: Make perlcritic happy This patch adds a .perlcriticrc (copied from qa-test-tools) and fixes almost all perlcrictic violations according to this .perlcriticrc The remaining violations are silenced out by appending a '## no critic' to the offending lines. They can still be seen by using the --force option of perlcritic This patch also modify t/00-testcritic.t to check all Perl files using the new .perlcriticrc. I'm not sure if this test script is still useful as it is now equivalent to `perlcritic --quiet .` and it looks like it is much slower (approximatively 5 times slower on my machine) Test plan: 1. Run `perlcritic --quiet .` from the root directory. It should output nothing 2. Run `perlcritic --quiet --force .`. It should output 7 errors (6 StringyEval, 1 BarewordFileHandles) 3. Run `TEST_QA=1 prove t/00-testcritic.t` 4. Read the patch. Check that all changes make sense and do not introduce undesired behaviour -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21395 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #3 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Hi Julian, an extensive change! All your test pass (1,2,3) but qa tools reports some errors, a lot of them are from tab chars, bout others are more real For example FAIL misc/migration_tools/22_to_30/export_Authorities_xml.pl OK critic OK forbidden patterns OK git manipulation OK pod OK spelling FAIL valid Global symbol "$time" requires explicit package name (did you forget to declare "my $time"?) Global symbol "$string" requires explicit package name (did you forget to declare "my $string"?) misc/migration_tools/22_to_30/export_Authorities_xml.pl had compilation errors. this is because of a missing ';' at the end of this line 34 my $string= ($time=~m/([0-9\-]+)/) ? $1 : undef Others need exploring. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21395 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #5 from Julian Maurice <julian.maurice@biblibre.com> --- I did not fix 'tab chars' errors, but all other errors should be gone -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21395 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #103666|0 |1 is obsolete| | --- Comment #6 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Created attachment 104038 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104038&action=edit Bug 21395: Make perlcritic happy This patch adds a .perlcriticrc (copied from qa-test-tools) and fixes almost all perlcrictic violations according to this .perlcriticrc The remaining violations are silenced out by appending a '## no critic' to the offending lines. They can still be seen by using the --force option of perlcritic This patch also modify t/00-testcritic.t to check all Perl files using the new .perlcriticrc. I'm not sure if this test script is still useful as it is now equivalent to `perlcritic --quiet .` and it looks like it is much slower (approximatively 5 times slower on my machine) Test plan: 1. Run `perlcritic --quiet .` from the root directory. It should output nothing 2. Run `perlcritic --quiet --force .`. It should output 7 errors (6 StringyEval, 1 BarewordFileHandles) 3. Run `TEST_QA=1 prove t/00-testcritic.t` 4. Read the patch. Check that all changes make sense and do not introduce undesired behaviour Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21395 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #8 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- (In reply to Julian Maurice from comment #5)

I did not fix 'tab chars' errors, but all other errors should be gone

Ok, only that remains are pre-existing tab chars and a small pod error Signed -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21395 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #104039|0 |1 is obsolete| | --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 106294 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106294&action=edit Bug 21395: Fix QA errors Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21395 --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 106296 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106296&action=edit Bug 21395: (QA follow-up) POD fixes Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21395 --- Comment #14 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> ---

This patch also modify t/00-testcritic.t to check all Perl files using the new .perlcriticrc. I'm not sure if this test script is still useful as it is now equivalent to `perlcritic --quiet .` and it looks like it is much slower (approximatively 5 times slower on my machine)

Are you sure about that? % time perlcritic --quiet . real 2m10.957s % time prove t/00-testcritic.t real 1m2.354s -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21395 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |20.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21395 --- Comment #17 from Julian Maurice <julian.maurice@biblibre.com> --- (In reply to Jonathan Druart from comment #14)

...

This patch also modify t/00-testcritic.t to check all Perl files using the new .perlcriticrc. I'm not sure if this test script is still useful as it is now equivalent to `perlcritic --quiet .` and it looks like it is much slower (approximatively 5 times slower on my machine)

Are you sure about that?

% time perlcritic --quiet . real 2m10.957s

% time prove t/00-testcritic.t real 1m2.354s

I just tested again, and I get similar times (t/00-testcritic.t is even faster (~40s)). It looks like t/00-testcritic.t uses multiple CPUs, unlike perlcritic. So t/00-testcritic.t is still useful :) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21395 --- Comment #19 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Also I think the changes to the migration_tools must be reverted unless they have been correctly tested. misc/migration_tools/22_to_30/export_Authorities_xml.pl and misc/migration_tools/22_to_30/move_marc_to_biblioitems.pl -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21395 --- Comment #20 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- And there are some other failures: * xt/author/translatable-templates.t * t/Makefile.t * t/00-testcritic.t * t/db_dependent/00-strict.t -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21395 --- Comment #22 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 106394 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106394&action=edit Bug 21395: Ignore .perlcriticrc in t/Makefile.t -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21395 --- Comment #24 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 106398 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106398&action=edit Bug 21395: Fix misc/admin/koha-preferences -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21395 --- Comment #26 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Thanks for the quick followups Julian.. great to see this one having progressed. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21395 Andrew Nugged <nugged@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nugged@gmail.com --- Comment #27 from Andrew Nugged <nugged@gmail.com> --- Small (below trivial :P) postfix: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25896 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21395 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|20.11.00 |20.11.00, 20.05.02 released in| | Status|Pushed to master |Pushed to stable CC| |lucas@bywatersolutions.com --- Comment #28 from Lucas Gass <lucas@bywatersolutions.com> --- backported to 20.05.x. for 20.05.02 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21395 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21395 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|additional_work_needed | -- You are receiving this mail because: You are watching all bug changes.