Bug 6679 - Fixing code so it passes basic Perl::Critic tests
Summary: Fixing code so it passes basic Perl::Critic tests
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P3 minor (vote)
Assignee: Jonathan Druart
QA Contact:
URL:
Keywords:
Depends on: 7119
Blocks: 7816 15680 15687
  Show dependency treegraph
 
Reported: 2011-08-08 08:20 UTC by Chris Cormack
Modified: 2017-06-14 22:11 UTC (History)
13 users (show)

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


Attachments
Bug 6679 : Fixing a style error to pass Perl::Critic, plus silencing a warn (1.56 KB, patch)
2011-08-08 08:34 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 6679 : Fixing a style error to pass Perl::Critic, plus silencing a warn (1.65 KB, patch)
2011-08-08 08:51 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 6679: More style errors, plus fixing a security issue (2.25 KB, patch)
2011-08-08 09:01 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 6679 : acqui/ now passes Perl::Critic tests (1.61 KB, patch)
2011-08-08 09:04 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 6679: Follow up patch explicitly using Carp (935 bytes, patch)
2011-08-08 09:10 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 6679 : scripts under admin style fixed (3.70 KB, patch)
2011-08-08 09:43 UTC, Chris Cormack
Details | Diff | Splinter Review
Signed-off patch (8.30 KB, patch)
2011-09-17 06:16 UTC, Frédéric Demians
Details | Diff | Splinter Review
Followup patch for acqui/histsearch.pl (1.67 KB, patch)
2011-09-17 06:17 UTC, Frédéric Demians
Details | Diff | Splinter Review
fix 3 :utf8 for :encoding(UTF-8) (1.92 KB, patch)
2011-09-20 21:47 UTC, Maxime Pelletier
Details | Diff | Splinter Review
Bug 6679 : Enable Critic tests for authorities and fixed resulting warnings (2.30 KB, patch)
2011-09-21 03:53 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 6679: Fixing admin/aqplain :encoding(UTF-8) gives strict validation (746 bytes, patch)
2011-10-31 07:58 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 6679: catalogue/ now passing perl critic tests (1.46 KB, patch)
2011-10-31 08:02 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 6679 : cataloguing/ now passing critic tests (3.76 KB, patch)
2011-10-31 08:18 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 6679 : Adding license statement and stopping declaring variables in a conditional (2.88 KB, patch)
2011-11-02 03:02 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 6679 : Declare variables outside of conditionals (4.03 KB, patch)
2011-11-02 03:02 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 6679 : Don't declare variables in conditionals Don't change $_ in a list context http://search.cpan.org/~elliotjs/Perl-Critic-1.116/lib/Perl/Critic/Policy/ControlStructures/ProhibitMutatingListFunctions.pm (2.44 KB, patch)
2011-11-02 03:14 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 6679 : Not using strict .. bad script, no cookie for you! (1001 bytes, patch)
2011-11-02 03:20 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 6679 : Don't declare variables in conditionals (3.38 KB, patch)
2011-11-02 03:24 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 6679 : Tidying changed parts of file (6.06 KB, patch)
2011-11-02 03:28 UTC, Chris Cormack
Details | Diff | Splinter Review
Reverting lines which break label-print.pl (1.31 KB, patch)
2011-11-09 19:58 UTC, Chris Nighswonger
Details | Diff | Splinter Review
Fixing critic complaint: Don't change $_ in a list context (1.22 KB, patch)
2011-11-09 20:50 UTC, Chris Nighswonger
Details | Diff | Splinter Review
Bug 6679 : Allowing eval require (586 bytes, patch)
2011-12-08 08:11 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 6697: fix missing use strict (5.89 KB, patch)
2011-12-08 16:14 UTC, Paul Poulain
Details | Diff | Splinter Review
Bug 6679: fix missing use strict (5.89 KB, patch)
2011-12-08 18:04 UTC, Chris Cormack
Details | Diff | Splinter Review
[3.6.x] Bug 6679: catalogue/ now passing perl critic tests (1.53 KB, patch)
2011-12-08 19:26 UTC, Chris Cormack
Details | Diff | Splinter Review
[3.6.x] Bug 6679 : cataloguing/ now passing critic tests (3.76 KB, patch)
2011-12-08 19:26 UTC, Chris Cormack
Details | Diff | Splinter Review
[3.6.x] Bug 6679 : Adding license statement and stopping declaring variables in a conditional (2.88 KB, patch)
2011-12-08 19:27 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 6679 : Fixing perlcritic violation (699 bytes, patch)
2012-03-22 20:04 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 6679 : Fixing perlcritic violation in authorities/ysearch.pl (790 bytes, patch)
2012-04-05 02:36 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 6679 members dir now passes perlcritic tests (2.21 KB, patch)
2012-04-05 02:43 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 6679 : Fixing some perlcritic violations in the opac (4.22 KB, patch)
2012-04-05 03:41 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 6679 :[SIGNED-OFF] members dir now passes perlcritic tests (2.29 KB, patch)
2012-04-06 12:42 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 : [SIGNED-OFF] Fixing some perlcritic violations in the opac (4.30 KB, patch)
2012-04-06 12:42 UTC, Jonathan Druart
Details | Diff | Splinter Review
Fixing 26 perlcritic violations in C4/Tags.pm (7.02 KB, patch)
2012-05-23 20:24 UTC, Mason James
Details | Diff | Splinter Review
Fixing 23 perlcritic violations in C4/Tags.pm (7.07 KB, patch)
2012-05-23 20:30 UTC, Mason James
Details | Diff | Splinter Review
Fixing 5 perlcritic violations in opac/* (2.97 KB, patch)
2012-05-23 20:45 UTC, Mason James
Details | Diff | Splinter Review
Fixing 24 perlcritic violations in misc/translator/TmplTokenizer.pm (4.67 KB, patch)
2012-05-23 20:58 UTC, Mason James
Details | Diff | Splinter Review
Fixing 13 perlcritic violations in C4/Barcodes.pm (3.78 KB, patch)
2012-05-23 21:02 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679: Fixing 26 perlcritic violations in C4/Tags.pm (6.64 KB, patch)
2012-06-09 19:38 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 6679: Fixing 23 perlcritic violations in C4/Tags.pm (7.12 KB, patch)
2012-06-09 19:38 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 6679: Fixing 5 perlcritic violations in opac/* (2.89 KB, patch)
2012-06-09 19:38 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 6679: Fixing 24 perlcritic violations in misc/translator/TmplTokenizer.pm (4.64 KB, patch)
2012-06-09 19:38 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 6679: Fixing 13 perlcritic violations in C4/Barcodes.pm (3.76 KB, patch)
2012-06-09 19:39 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6679: Fixing 13 perlcritic violations in C4/Barcodes.pm (3.79 KB, patch)
2012-06-10 10:49 UTC, Joy Nelson
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6679: Fixing 24 perlcritic violations in misc/translator/TmplTokenizer.pm (4.67 KB, patch)
2012-06-10 10:53 UTC, Joy Nelson
Details | Diff | Splinter Review
[SIGNED OFF] Bug 6679: Fixing 5 perlcritic violations in opac/* (2.91 KB, patch)
2012-06-10 10:54 UTC, Joy Nelson
Details | Diff | Splinter Review
[SIGNED OFF] Bug 6679: Fixing 23 perlcritic violations in C4/Tags.pm (7.14 KB, patch)
2012-06-10 10:55 UTC, Joy Nelson
Details | Diff | Splinter Review
[SIGNED OFF] Bug 6679: Fixing 26 perlcritic violations in C4/Tags.pm (6.66 KB, patch)
2012-06-10 10:56 UTC, Joy Nelson
Details | Diff | Splinter Review
Bug 6679 - fixing 23 '"return" statement with explicit "undef"' violations in C4/Tags.pm (6.90 KB, patch)
2012-06-26 04:08 UTC, Mason James
Details | Diff | Splinter Review
[FAILED-QA]Bug 6679 - fix 19 perlcritic violations from C4::Letters.pm (4.79 KB, patch)
2012-06-26 04:16 UTC, Mason James
Details | Diff | Splinter Review
fix 15 perlcritic violations in C4/ImportExportFramework.pm (3.90 KB, patch)
2012-06-26 04:40 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - fix 12 perlcritic violations in C4/Context.pm (3.23 KB, patch)
2012-06-26 12:36 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - fix 12 perlcritic violations in C4/Auth_with_ldap.pm (3.20 KB, patch)
2012-06-26 12:40 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - fix 12 perlcritic violations in C4/External/BakerTaylor.pm (2.33 KB, patch)
2012-06-26 12:44 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - fix 11 perlcritic violations in C4/Dates.pm (3.94 KB, patch)
2012-06-26 13:07 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - fix 9 perlcritic violations in C4/TmplTokenType.pm (1.28 KB, patch)
2012-06-26 13:09 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - fix 9 perlcritic violations in C4/Koha.pm (2.59 KB, patch)
2012-06-26 13:12 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF]fix 12 perlcritic violations in C4/External/BakerTaylor.pm (2.32 KB, patch)
2012-06-26 13:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 12 perlcritic violations in C4/Auth_with_ldap.pm (3.21 KB, patch)
2012-06-26 13:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 -[SIGNED-OFF] fix 12 perlcritic violations in C4/Context.pm (3.23 KB, patch)
2012-06-26 13:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF]fixing 23 '"return" statement with explicit "undef"' violations in C4/Tags.pm (6.91 KB, patch)
2012-06-26 13:15 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - fix 8 perlcritic violations in C4/Circulation.pm (2.84 KB, patch)
2012-06-26 13:21 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - fix 8 perlcritic violations in C4/Barcodes.pm (2.22 KB, patch)
2012-06-26 13:26 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - fix 7 perlcritic violations in C4/Biblio.pm (2.33 KB, patch)
2012-06-26 13:44 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - fix 6 perlcritic violations in C4/Reports/Guided.pm (2.41 KB, patch)
2012-06-26 13:51 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - fix 6 perlcritic violations in C4/Barcodes/hbyymmincr.pm (2.37 KB, patch)
2012-06-26 13:54 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - fix 5 perlcritic violations in C4/VirtualShelves.pm, and comment-out obselete sub() (2.17 KB, patch)
2012-06-26 13:59 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - fix 5 perlcritic violations in C4/Serials.pm (1.72 KB, patch)
2012-06-26 14:03 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - fix 4 perlcritic violations in C4/ShelfBrowser.pm (1.58 KB, patch)
2012-06-26 14:11 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - fix 4 perlcritic violations in C4/Installer.pm (1.64 KB, patch)
2012-06-26 14:23 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - fix 4 perlcritic violations in C4/Creators/PDF.pm (2.73 KB, patch)
2012-06-26 14:29 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - fix 4 perlcritic violations in C4/Branch.pm (2.05 KB, patch)
2012-06-26 14:31 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - fix 3 perlcritic violations in C4/SQLHelper.pm (1.43 KB, patch)
2012-06-26 14:39 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - fix 3 perlcritic violations in C4/Record.pm (1.40 KB, patch)
2012-06-26 14:42 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - fix 3 perlcritic violations in C4/Output.pm (1.77 KB, patch)
2012-06-26 14:45 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - fix 3 perlcritic violations in C4/Message.pm (1.36 KB, patch)
2012-06-26 14:48 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - fix 3 perlcritic violations in C4/Items.pm (1.93 KB, patch)
2012-06-26 14:52 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - [FAILED QA]fix 3 perlcritic violations in C4/ClassSortRoutine.pm (1.48 KB, patch)
2012-06-26 14:56 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - fix 3 perlcritic violations in C4/Auth.pm, and enabled warnings (1.85 KB, patch)
2012-06-26 20:37 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - fix 2 perlcritic violations in C4/Utils.pm (906 bytes, patch)
2012-06-26 20:37 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - fix 2 perlcritic violations in C4/Print.pm (1.70 KB, patch)
2012-06-26 20:38 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - fix 2 perlcritic violations in C4/ItemCirculationAlertPreference.pm (1.17 KB, patch)
2012-06-26 20:38 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - fix 2 perlcritic violations in C4/Installer/PerlModules.pm (1.63 KB, patch)
2012-06-26 20:38 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - fix 2 perlcritic violations in C4/BackgroundJob.pm (1.22 KB, patch)
2012-06-26 20:38 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - fix 2 perlcritic violations in C4/AuthoritiesMarc.pm (1.53 KB, patch)
2012-06-26 20:38 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 15 perlcritic violations in C4/ImportExportFramework.pm (3.92 KB, patch)
2012-07-18 13:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 11 perlcritic violations in C4/Dates.pm (3.95 KB, patch)
2012-07-18 13:12 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 9 perlcritic violations in C4/TmplTokenType.pm (1.29 KB, patch)
2012-07-18 13:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 9 perlcritic violations in C4/Koha.pm (2.73 KB, patch)
2012-07-18 13:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 8 perlcritic violations in C4/Circulation.pm (2.78 KB, patch)
2012-07-18 13:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 8 perlcritic violations in C4/Barcodes.pm (2.16 KB, patch)
2012-07-18 13:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 8 perlcritic violations in C4/Circulation.pm (2.86 KB, patch)
2012-07-18 13:51 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 8 perlcritic violations in C4/Barcodes.pm (2.23 KB, patch)
2012-07-18 13:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 7 perlcritic violations in C4/Biblio.pm (2.35 KB, patch)
2012-07-18 13:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 6 perlcritic violations in C4/Reports/Guided.pm (2.42 KB, patch)
2012-07-18 13:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 6 perlcritic violations in C4/Barcodes/hbyymmincr.pm (2.38 KB, patch)
2012-07-18 13:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 5 perlcritic violations in C4/Serials.pm (1.74 KB, patch)
2012-07-18 13:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 4 perlcritic violations in C4/ShelfBrowser.pm (1.59 KB, patch)
2012-07-18 13:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 4 perlcritic violations in C4/Branch.pm (2.06 KB, patch)
2012-07-18 13:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 3 perlcritic violations in C4/SQLHelper.pm (1.45 KB, patch)
2012-07-18 13:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 3 perlcritic violations in C4/Record.pm (1.42 KB, patch)
2012-07-18 13:57 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 3 perlcritic violations in C4/Output.pm (1.78 KB, patch)
2012-07-18 13:57 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 3 perlcritic violations in C4/Message.pm (1.37 KB, patch)
2012-07-18 13:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 3 perlcritic violations in C4/Items.pm (1.94 KB, patch)
2012-07-18 13:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 3 perlcritic violations in C4/ClassSortRoutine.pm (1.50 KB, patch)
2012-07-18 13:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 2 perlcritic violations in C4/Utils.pm (921 bytes, patch)
2012-07-18 14:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 2 perlcritic violations in C4/ItemCirculationAlertPreference.pm (1.18 KB, patch)
2012-07-18 14:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 2 perlcritic violations in C4/Installer/PerlModules.pm (1.65 KB, patch)
2012-07-18 14:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 2 perlcritic violations in C4/BackgroundJob.pm (1.24 KB, patch)
2012-07-18 14:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 2 perlcritic violations in C4/AuthoritiesMarc.pm (1.55 KB, patch)
2012-07-18 14:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 5 perlcritic violations in C4/VirtualShelves.pm, and comment-out obselete sub() (2.19 KB, patch)
2012-07-18 14:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 4 perlcritic violations in C4/Installer.pm (1.80 KB, patch)
2012-07-18 14:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 4 perlcritic violations in C4/Creators/PDF.pm (2.75 KB, patch)
2012-07-18 14:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 2 perlcritic violations in C4/Print.pm (1.96 KB, patch)
2012-07-18 14:21 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 3 perlcritic violations in C4/Auth.pm, and enabled warnings (1.96 KB, patch)
2012-07-18 14:30 UTC, Jonathan Druart
Details | Diff | Splinter Review
Proposed patch (1.54 KB, patch)
2012-09-14 13:06 UTC, Colin Campbell
Details | Diff | Splinter Review
Proposed patch to calm Jenkins (1.47 KB, patch)
2012-09-19 16:30 UTC, Colin Campbell
Details | Diff | Splinter Review
Bug 6679 - Fixing code so it passes basic Perl:::Critic tests (998 bytes, patch)
2012-09-24 23:11 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - Fixing code so it passes basic Perl:::Critic tests (995 bytes, patch)
2012-09-24 23:16 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - Fixing code so it passes basic Perl:::Critic tests (1.27 KB, patch)
2012-09-24 23:22 UTC, Mason James
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 11 perlcritic violations in C4/Dates.pm (3.95 KB, patch)
2014-12-09 08:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 9 perlcritic violations in C4/TmplTokenType.pm (1.28 KB, patch)
2014-12-09 08:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 6 perlcritic violations in C4/Reports/Guided.pm (1.25 KB, patch)
2014-12-09 08:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 3 perlcritic violations in C4/Record.pm (1.36 KB, patch)
2014-12-09 08:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 2 perlcritic violations in C4/Installer/PerlModules.pm (1.64 KB, patch)
2014-12-09 08:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - Fixing code so it passes basic Perl:::Critic tests (1.19 KB, patch)
2014-12-09 08:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679: (follow-up) fix 9 perlcritic violations in C4/TmplTokenType.pm (1.73 KB, patch)
2014-12-09 08:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6679 - Fixing code so it passes basic Perl:::Critic tests (1.29 KB, patch)
2015-05-12 17:30 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6679: (follow-up) fix 9 perlcritic violations in C4/TmplTokenType.pm (1.81 KB, patch)
2015-05-12 17:30 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 3 perlcritic violations in C4/Record.pm (1.70 KB, patch)
2015-05-12 17:32 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[PASSED QA] Bug 6679 - [SIGNED-OFF] fix 11 perlcritic violations in C4/Dates.pm (4.02 KB, patch)
2015-05-22 12:25 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 6679 - [SIGNED-OFF] fix 9 perlcritic violations in C4/TmplTokenType.pm (1.35 KB, patch)
2015-05-22 12:26 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 6679 - [SIGNED-OFF] fix 6 perlcritic violations in C4/Reports/Guided.pm (1.32 KB, patch)
2015-05-22 12:26 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 6679 - [SIGNED-OFF] fix 2 perlcritic violations in C4/Installer/PerlModules.pm (1.71 KB, patch)
2015-05-22 12:26 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 6679 - Fixing code so it passes basic Perl:::Critic tests (1.34 KB, patch)
2015-05-22 12:26 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 6679: (follow-up) fix 9 perlcritic violations in C4/TmplTokenType.pm (1.86 KB, patch)
2015-05-22 12:26 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 6679 - [SIGNED-OFF] fix 3 perlcritic violations in C4/Record.pm (1.77 KB, patch)
2015-05-22 12:26 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 6679: Fix previous change to C4::Dates (834 bytes, patch)
2015-09-22 11:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 9 perlcritic violations in C4/TmplTokenType.pm (1.34 KB, patch)
2016-01-21 08:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 2 perlcritic violations in C4/Installer/PerlModules.pm (1.69 KB, patch)
2016-01-21 08:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - Fixing code so it passes basic Perl:::Critic tests (1.33 KB, patch)
2016-01-21 08:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679: (follow-up) fix 9 perlcritic violations in C4/TmplTokenType.pm (1.84 KB, patch)
2016-01-21 08:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 3 perlcritic violations in C4/Record.pm (1.75 KB, patch)
2016-01-21 08:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 9 perlcritic violations in C4/TmplTokenType.pm (1.39 KB, patch)
2016-01-22 13:59 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 6679 - [SIGNED-OFF] fix 2 perlcritic violations in C4/Installer/PerlModules.pm (1.74 KB, patch)
2016-01-22 13:59 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 6679 - Fixing code so it passes basic Perl:::Critic tests (1.38 KB, patch)
2016-01-22 13:59 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 6679: (follow-up) fix 9 perlcritic violations in C4/TmplTokenType.pm (1.89 KB, patch)
2016-01-22 14:00 UTC, Mark Tompsett
Details | Diff | Splinter Review
[PASSED QA] Bug 6679 - [SIGNED-OFF] fix 9 perlcritic violations in C4/TmplTokenType.pm (1.46 KB, patch)
2016-01-22 17:34 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 6679 - [SIGNED-OFF] fix 2 perlcritic violations in C4/Installer/PerlModules.pm (1.81 KB, patch)
2016-01-22 17:34 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 6679 - Fixing code so it passes basic Perl:::Critic tests (1.44 KB, patch)
2016-01-22 17:34 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 6679: (follow-up) fix 9 perlcritic violations in C4/TmplTokenType.pm (1.96 KB, patch)
2016-01-22 17:34 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 Chris Cormack 2011-08-08 08:20:45 UTC
This bug will be an ongoing one, until all our code passes perl critic tests
Comment 1 Chris Cormack 2011-08-08 08:34:43 UTC Comment hidden (obsolete)
Comment 2 Chris Cormack 2011-08-08 08:51:47 UTC Comment hidden (obsolete)
Comment 3 Chris Cormack 2011-08-08 09:01:09 UTC Comment hidden (obsolete)
Comment 4 Chris Cormack 2011-08-08 09:04:50 UTC Comment hidden (obsolete)
Comment 5 Chris Cormack 2011-08-08 09:10:51 UTC Comment hidden (obsolete)
Comment 6 Chris Cormack 2011-08-08 09:43:04 UTC Comment hidden (obsolete)
Comment 7 Frédéric Demians 2011-09-17 06:16:10 UTC Comment hidden (obsolete)
Comment 8 Frédéric Demians 2011-09-17 06:17:01 UTC Comment hidden (obsolete)
Comment 9 Maxime Pelletier 2011-09-20 21:47:43 UTC Comment hidden (obsolete)
Comment 10 Chris Cormack 2011-09-21 03:00:17 UTC
Patches pushed, leaving bug open to catch more 

Now passing for 130 files
Comment 11 Chris Cormack 2011-09-21 03:53:31 UTC Comment hidden (obsolete)
Comment 12 Ian Walls 2011-09-22 12:06:03 UTC
Passing latest patch through QA (also consider this my sign off)
Comment 13 Chris Cormack 2011-09-23 02:19:55 UTC
All pushed, leaving open for me
Comment 14 Chris Cormack 2011-10-31 07:58:20 UTC Comment hidden (obsolete)
Comment 15 Chris Cormack 2011-10-31 08:02:47 UTC Comment hidden (obsolete)
Comment 16 Chris Cormack 2011-10-31 08:18:42 UTC Comment hidden (obsolete)
Comment 17 Chris Cormack 2011-11-02 03:02:32 UTC Comment hidden (obsolete)
Comment 18 Chris Cormack 2011-11-02 03:02:43 UTC Comment hidden (obsolete)
Comment 19 Chris Cormack 2011-11-02 03:14:18 UTC Comment hidden (obsolete)
Comment 20 Chris Cormack 2011-11-02 03:20:28 UTC Comment hidden (obsolete)
Comment 21 Chris Cormack 2011-11-02 03:24:43 UTC Comment hidden (obsolete)
Comment 22 Chris Cormack 2011-11-02 03:28:31 UTC Comment hidden (obsolete)
Comment 23 Paul Poulain 2011-11-06 17:04:15 UTC
The 2nd patch does not work well I think (it also does not apply, but it's easily fixed)

in showmarc.pl, before your patch, my biblio with diacritic look like :
200 1  _aMassacre (The) éô ï
(éô ï is what i'm expecting)
After the patch, it looks like :
200 1  _aMassacre (The) éô ï
which is the sign of a double encoded biblio record.

Marking "failed QA" for now. The 3rd patch, that updates perlcritic.tt does not apply after that (i've not tested further, I suspect they will conflict because catalogue has not been added

Once the showmarc issue is solved, i'll test & push those usefull code cleaning patches as fast as possible
Comment 24 Paul Poulain 2011-11-06 17:20:22 UTC
Comment on attachment 6144 [details] [review]
Bug 6679 : Tidying changed parts of file

this patch has been pushed
Comment 25 Paul Poulain 2011-11-06 17:20:38 UTC
Comment on attachment 6143 [details] [review]
Bug 6679 : Don't declare variables in conditionals

this patch has been pushed
Comment 26 Paul Poulain 2011-11-06 17:20:59 UTC
Comment on attachment 6142 [details] [review]
Bug 6679 : Not using strict .. bad script, no cookie for you!

this patch has been pushed
Comment 27 Paul Poulain 2011-11-06 17:21:17 UTC
Comment on attachment 6141 [details] [review]
Bug 6679 : Don't declare variables in conditionals Don't change $_ in a list context http://search.cpan.org/~elliotjs/Perl-Critic-1.116/lib/Perl/Critic/Policy/ControlStructures/ProhibitMutatingListFunctions.pm

this patch has been pushed
Comment 28 Paul Poulain 2011-11-06 17:21:32 UTC
Comment on attachment 6140 [details] [review]
Bug 6679 : Declare variables outside of conditionals

This patch has been pushed
Comment 29 Paul Poulain 2011-11-06 17:30:21 UTC
Comment on attachment 6121 [details] [review]
Bug 6679: Fixing admin/aqplain :encoding(UTF-8) gives strict validation

this patch has been pushed
Comment 30 Chris Nighswonger 2011-11-09 19:58:13 UTC Comment hidden (obsolete)
Comment 31 Chris Nighswonger 2011-11-09 20:50:57 UTC Comment hidden (obsolete)
Comment 32 Chris Cormack 2011-12-08 08:11:21 UTC Comment hidden (obsolete)
Comment 33 Paul Poulain 2011-12-08 09:32:38 UTC
Comment on attachment 6122 [details] [review]
Bug 6679: catalogue/ now passing perl critic tests

patch pushed
Comment 34 Paul Poulain 2011-12-08 09:34:10 UTC
Comment on attachment 6123 [details] [review]
Bug 6679 : cataloguing/ now passing critic tests

patch pushed.

trying to prove cataloguing show me some errors in plugins though:

prove t/00-testcritic.t 
t/00-testcritic.t .. 196/246 
#   Failed test 'Test::Perl::Critic for "cataloguing/value_builder/stocknumber.pl"'
#   at /usr/share/perl5/Test/Perl/Critic.pm line 110.
# 
# Perl::Critic found these violations in "cataloguing/value_builder/stocknumber.pl":
# Code before strictures are enabled at line 30, column 1.  See page 429 of PBP.  (Severity: 5)
t/00-testcritic.t .. 239/246 
#   Failed test 'Test::Perl::Critic for "cataloguing/value_builder/unimarc_field_210c.pl"'
#   at /usr/share/perl5/Test/Perl/Critic.pm line 110.
# 
# Perl::Critic found these violations in "cataloguing/value_builder/unimarc_field_210c.pl":
# Code before strictures are enabled at line 40, column 1.  See page 429 of PBP.  (Severity: 5)
# Variable declared in conditional statement at line 113, column 9.  Declare variables outside of the condition.  (Severity: 5)
t/00-testcritic.t .. 243/246 
#   Failed test 'Test::Perl::Critic for "cataloguing/value_builder/unimarc_field_4XX.pl"'
#   at /usr/share/perl5/Test/Perl/Critic.pm line 110.
# 
# Perl::Critic found these violations in "cataloguing/value_builder/unimarc_field_4XX.pl":
# Variable declared in conditional statement at line 272, column 9.  Declare variables outside of the condition.  (Severity: 5)
# Variable declared in conditional statement at line 286, column 9.  Declare variables outside of the condition.  (Severity: 5)
# Variable declared in conditional statement at line 292, column 9.  Declare variables outside of the condition.  (Severity: 5)
# Variable declared in conditional statement at line 377, column 13.  Declare variables outside of the condition.  (Severity: 5)
t/00-testcritic.t .. 245/246 # Looks like you failed 3 tests of 246.


I think it's worth activating the test (it's better to have a test showing something is wrong than doing no test and thinking things are OK ;-) )
Comment 35 Paul Poulain 2011-12-08 09:34:24 UTC
Comment on attachment 6139 [details] [review]
Bug 6679 : Adding license statement and stopping declaring variables in a conditional

patch pushed
Comment 36 Paul Poulain 2011-12-08 09:34:34 UTC
Comment on attachment 6663 [details] [review]
Bug 6679 : Allowing eval require

patch pushed
Comment 37 Paul Poulain 2011-12-08 09:37:49 UTC
Comment on attachment 6270 [details] [review]
Reverting lines which break label-print.pl

patch already applied (bug 7207)
Comment 38 Paul Poulain 2011-12-08 09:38:20 UTC
Comment on attachment 6271 [details] [review]
Fixing critic complaint: Don't change $_ in a list context

patch already applied (bug 7207)
Comment 39 Chris Nighswonger 2011-12-08 16:11:50 UTC
The latest round of patches for this bug do not apply cleanly to 3.6.x. Please attach corrected patches to this bug report.
Comment 40 Paul Poulain 2011-12-08 16:14:24 UTC Comment hidden (obsolete)
Comment 41 Chris Cormack 2011-12-08 17:57:27 UTC
Which patches chris_n .. they have been marked obsolete and I have lost track of them,

If you list the ones you need ill rebase them.
Comment 42 Chris Cormack 2011-12-08 18:04:41 UTC Comment hidden (obsolete)
Comment 43 Chris Nighswonger 2011-12-08 19:14:15 UTC
(In reply to comment #41)
> Which patches chris_n .. they have been marked obsolete and I have lost track
> of them,
> 
> If you list the ones you need ill rebase them.

Here are the commit ids:

4a14e7dbe69db0517d1d333a644c4abeb0a42cb8
a047dea5e46338d3d664935451764fffe56e6d9e
dc6e1031f8c2aa36e88ca39a70c5b7d9fd63065d
Comment 44 Chris Cormack 2011-12-08 19:26:14 UTC Comment hidden (obsolete)
Comment 45 Chris Cormack 2011-12-08 19:26:46 UTC Comment hidden (obsolete)
Comment 46 Chris Cormack 2011-12-08 19:27:12 UTC Comment hidden (obsolete)
Comment 47 Paul Poulain 2011-12-09 08:52:12 UTC
Comment on attachment 6675 [details] [review]
Bug 6679: fix missing use strict

patch pushed
Comment 48 Paul Poulain 2011-12-13 14:15:27 UTC
Chris_n, please apply those patches, then back to "priority 1" and no patch status, until more tests are provided
Comment 49 Chris Nighswonger 2011-12-13 14:26:30 UTC
These patches were applied to 3.6.x last week.
Comment 50 Chris Cormack 2012-03-22 20:04:20 UTC Comment hidden (obsolete)
Comment 51 Paul Poulain 2012-03-23 09:38:20 UTC
trivial patch
Comment 52 Paul Poulain 2012-03-23 09:38:51 UTC
Comment on attachment 8542 [details] [review]
Bug 6679 : Fixing perlcritic violation

this patch has been pushed today
Comment 53 Chris Cormack 2012-04-05 02:36:53 UTC Comment hidden (obsolete)
Comment 54 Chris Cormack 2012-04-05 02:43:51 UTC Comment hidden (obsolete)
Comment 55 Chris Cormack 2012-04-05 03:41:34 UTC Comment hidden (obsolete)
Comment 56 Jonathan Druart 2012-04-06 12:42:21 UTC Comment hidden (obsolete)
Comment 57 Jonathan Druart 2012-04-06 12:42:56 UTC Comment hidden (obsolete)
Comment 58 Jonathan Druart 2012-04-06 12:45:08 UTC
Comment on attachment 8897 [details] [review]
Bug 6679 : Fixing perlcritic violation in authorities/ysearch.pl

I signed off the same patch (from Colin) today. (Bug 7816)
Comment 59 Ian Walls 2012-04-06 22:17:52 UTC
Both signed off patches faithfully replace functionality with Perl::Critic compliant code.  Marking as Passed QA.
Comment 60 Paul Poulain 2012-04-10 11:45:29 UTC
Comment on attachment 8950 [details] [review]
Bug 6679 :[SIGNED-OFF] members dir now passes perlcritic tests

This patch has been pushed
Comment 61 Paul Poulain 2012-04-10 11:46:20 UTC
Comment on attachment 8951 [details] [review]
Bug 6679 : [SIGNED-OFF] Fixing some perlcritic violations in the opac

This patch has been pushed

A few scripts still not perlcritic compliant in opac/ directory:
opac/changelanguage.pl: Code before strictures are enabled at line 24, column 1.  See page 429 of PBP.  (Severity: 5)
opac/ilsdi.pl: Stricture disabled at line 217, column 17.  See page 429 of PBP.  (Severity: 5)
opac/opac-detail.pl: Variable declared in conditional statement at line 139, column 9.  Declare variables outside of the condition.  (Severity: 5)
opac/opac-detail.pl: Variable declared in conditional statement at line 140, column 9.  Declare variables outside of the condition.  (Severity: 5)
opac/opac-detail.pl: Variable declared in conditional statement at line 416, column 1.  Declare variables outside of the condition.  (Severity: 5)
opac/opac-reserve.pl: Subroutine prototypes used at line 51, column 1.  See page 194 of PBP.  (Severity: 5)
Comment 62 Mason James 2012-05-23 20:24:39 UTC Comment hidden (obsolete)
Comment 63 Mason James 2012-05-23 20:30:31 UTC Comment hidden (obsolete)
Comment 64 Mason James 2012-05-23 20:45:16 UTC Comment hidden (obsolete)
Comment 65 Mason James 2012-05-23 20:58:01 UTC Comment hidden (obsolete)
Comment 66 Mason James 2012-05-23 21:02:35 UTC Comment hidden (obsolete)
Comment 67 Jonathan Druart 2012-05-29 14:07:43 UTC
Fixing 26 perlcritic violations in C4/Tags.pm
Apply? [yn] y

Applying: Fixing 26 perlcritic violations in C4/Tags.pm
/home/jonathan/workspace/versions/koha_coderun/.git/rebase-apply/patch:32: trailing whitespace.
sub approval_counts { 
error: patch failed: opac/opac-tags.pl:43
error: opac/opac-tags.pl: patch does not apply
Patch failed at 0001 Fixing 26 perlcritic violations in C4/Tags.pm
Comment 68 Jared Camins-Esakov 2012-06-09 19:38:10 UTC Comment hidden (obsolete)
Comment 69 Jared Camins-Esakov 2012-06-09 19:38:31 UTC Comment hidden (obsolete)
Comment 70 Jared Camins-Esakov 2012-06-09 19:38:43 UTC Comment hidden (obsolete)
Comment 71 Jared Camins-Esakov 2012-06-09 19:38:54 UTC Comment hidden (obsolete)
Comment 72 Jared Camins-Esakov 2012-06-09 19:39:10 UTC Comment hidden (obsolete)
Comment 73 Jared Camins-Esakov 2012-06-09 19:39:40 UTC
All patches rebased on latest master. Signoff needed. :)
Comment 74 Joy Nelson 2012-06-10 10:49:41 UTC Comment hidden (obsolete)
Comment 75 Joy Nelson 2012-06-10 10:53:57 UTC Comment hidden (obsolete)
Comment 76 Joy Nelson 2012-06-10 10:54:49 UTC Comment hidden (obsolete)
Comment 77 Joy Nelson 2012-06-10 10:55:47 UTC Comment hidden (obsolete)
Comment 78 Joy Nelson 2012-06-10 10:56:31 UTC Comment hidden (obsolete)
Comment 79 Joy Nelson 2012-06-10 10:59:23 UTC
FYI - Still additional PBP violations in TmplTokenizer.pm and C4/Barcodes.pm
Comment 80 Paul Poulain 2012-06-10 13:06:31 UTC
Comment on attachment 10075 [details] [review]
[SIGNED-OFF] Bug 6679: Fixing 13 perlcritic violations in C4/Barcodes.pm

OK, this patch fixes error, but there are still some:
"return" statement with explicit "undef" at line 57, column 2.  See page 199 of PBP.  (Severity: 5)
"return" statement with explicit "undef" at line 131, column 3.  See page 199 of PBP.  (Severity: 5)
"return" statement with explicit "undef" at line 147, column 22.  See page 199 of PBP.  (Severity: 5)
"return" statement with explicit "undef" at line 152, column 22.  See page 199 of PBP.  (Severity: 5)
"return" statement with explicit "undef" at line 157, column 22.  See page 199 of PBP.  (Severity: 5)
"return" statement with explicit "undef" at line 193, column 3.  See page 199 of PBP.  (Severity: 5)
"return" statement with explicit "undef" at line 197, column 3.  See page 199 of PBP.  (Severity: 5)
"return" statement with explicit "undef" at line 228, column 2.  See page 199 of PBP.  (Severity: 5)
Comment 81 Paul Poulain 2012-06-10 13:11:16 UTC
Comment on attachment 10076 [details] [review]
[SIGNED-OFF] Bug 6679: Fixing 24 perlcritic violations in misc/translator/TmplTokenizer.pm

Patch pushed, still one perlcritic problem in this file:
"return" statement with explicit "undef" at line 319, column 17.  See page 199 of PBP.  (Severity: 5)
Comment 82 Paul Poulain 2012-06-10 13:12:38 UTC
Comment on attachment 10077 [details] [review]
[SIGNED OFF] Bug 6679: Fixing 5 perlcritic violations in opac/*

patch pushed,OPAC almost perlcritic compliant, just one error to solve:
opac/ilsdi.pl: Stricture disabled at line 217, column 17.  See page 429 of PBP.  (Severity: 5)
Comment 83 Paul Poulain 2012-06-10 13:13:20 UTC
Comment on attachment 10078 [details] [review]
[SIGNED OFF] Bug 6679: Fixing 23 perlcritic violations in C4/Tags.pm

I can't apply this one:
Applying: Bug 6679: Fixing 23 perlcritic violations in C4/Tags.pm
fatal: sha1 information is lacking or useless (C4/Tags.pm).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 6679: Fixing 23 perlcritic violations in C4/Tags.pm
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".
Comment 84 Paul Poulain 2012-06-10 13:14:29 UTC
Comment on attachment 10078 [details] [review]
[SIGNED OFF] Bug 6679: Fixing 23 perlcritic violations in C4/Tags.pm

This patch does not apply, replaced by another one
Comment 85 Paul Poulain 2012-06-10 13:15:58 UTC
Comment on attachment 10079 [details] [review]
[SIGNED OFF] Bug 6679: Fixing 26 perlcritic violations in C4/Tags.pm

this patch has been pushed
Comment 86 Paul Poulain 2012-06-10 13:16:47 UTC
(In reply to comment #85)
> Comment on attachment 10079 [details] [review]
> [SIGNED OFF] Bug 6679: Fixing 26 perlcritic violations in C4/Tags.pm
> 
> this patch has been pushed

Still a lot of things to update in C4/Tags.pm:
"return" statement with explicit "undef" at line 120, column 25.  See page 199 of PBP.  (Severity: 5)
"return" statement with explicit "undef" at line 126, column 27.  See page 199 of PBP.  (Severity: 5)
"return" statement with explicit "undef" at line 148, column 10.  See page 199 of PBP.  (Severity: 5)
"return" statement with explicit "undef" at line 154, column 10.  See page 199 of PBP.  (Severity: 5)
"return" statement with explicit "undef" at line 160, column 10.  See page 199 of PBP.  (Severity: 5)
"return" statement with explicit "undef" at line 166, column 10.  See page 199 of PBP.  (Severity: 5)
"return" statement with explicit "undef" at line 362, column 22.  See page 199 of PBP.  (Severity: 5)
"return" statement with explicit "undef" at line 373, column 22.  See page 199 of PBP.  (Severity: 5)
"return" statement with explicit "undef" at line 387, column 23.  See page 199 of PBP.  (Severity: 5)
"return" statement with explicit "undef" at line 409, column 23.  See page 199 of PBP.  (Severity: 5)
"return" statement with explicit "undef" at line 422, column 23.  See page 199 of PBP.  (Severity: 5)
"return" statement with explicit "undef" at line 429, column 23.  See page 199 of PBP.  (Severity: 5)
"return" statement with explicit "undef" at line 438, column 22.  See page 199 of PBP.  (Severity: 5)
"return" statement with explicit "undef" at line 463, column 23.  See page 199 of PBP.  (Severity: 5)
"return" statement with explicit "undef" at line 464, column 26.  See page 199 of PBP.  (Severity: 5)
"return" statement with explicit "undef" at line 473, column 30.  See page 199 of PBP.  (Severity: 5)
"return" statement with explicit "undef" at line 474, column 30.  See page 199 of PBP.  (Severity: 5)
"return" statement with explicit "undef" at line 487, column 10.  See page 199 of PBP.  (Severity: 5)
"return" statement with explicit "undef" at line 508, column 47.  See page 199 of PBP.  (Severity: 5)
"return" statement with explicit "undef" at line 560, column 30.  See page 199 of PBP.  (Severity: 5)
"return" statement with explicit "undef" at line 561, column 30.  See page 199 of PBP.  (Severity: 5)
"return" statement with explicit "undef" at line 565, column 13.  See page 199 of PBP.  (Severity: 5)
"return" statement with explicit "undef" at line 574, column 3.  See page 199 of PBP.  (Severity: 5)
Comment 87 Mason James 2012-06-26 04:08:01 UTC Comment hidden (obsolete)
Comment 88 Mason James 2012-06-26 04:16:19 UTC Comment hidden (obsolete)
Comment 89 Mason James 2012-06-26 04:40:30 UTC Comment hidden (obsolete)
Comment 90 Mason James 2012-06-26 12:36:33 UTC Comment hidden (obsolete)
Comment 91 Mason James 2012-06-26 12:40:01 UTC Comment hidden (obsolete)
Comment 92 Mason James 2012-06-26 12:44:11 UTC Comment hidden (obsolete)
Comment 93 Mason James 2012-06-26 13:07:30 UTC Comment hidden (obsolete)
Comment 94 Mason James 2012-06-26 13:09:24 UTC Comment hidden (obsolete)
Comment 95 Mason James 2012-06-26 13:12:45 UTC Comment hidden (obsolete)
Comment 96 Jonathan Druart 2012-06-26 13:13:20 UTC Comment hidden (obsolete)
Comment 97 Jonathan Druart 2012-06-26 13:14:12 UTC Comment hidden (obsolete)
Comment 98 Jonathan Druart 2012-06-26 13:14:55 UTC Comment hidden (obsolete)
Comment 99 Jonathan Druart 2012-06-26 13:15:42 UTC Comment hidden (obsolete)
Comment 100 Jonathan Druart 2012-06-26 13:19:29 UTC
(In reply to comment #88)
> Created attachment 10485 [details] [review]
> Bug 6679 - fix 19 perlcritic violations from C4::Letters.pm


FAILED QA for this patch, I think the following lines don't have to be modified:
-    return unless exists $params->{'letter'};
-    return unless exists $params->{'borrowernumber'};
-    return unless exists $params->{'message_transport_type'};
+    return exists $params->{'letter'};
+    return exists $params->{'borrowernumber'};
+    return exists $params->{'message_transport_type'};

It's unless, not undef :-)
Comment 101 Mason James 2012-06-26 13:21:32 UTC Comment hidden (obsolete)
Comment 102 Mason James 2012-06-26 13:26:28 UTC Comment hidden (obsolete)
Comment 103 Mason James 2012-06-26 13:44:40 UTC Comment hidden (obsolete)
Comment 104 Mason James 2012-06-26 13:51:48 UTC Comment hidden (obsolete)
Comment 105 Mason James 2012-06-26 13:54:33 UTC Comment hidden (obsolete)
Comment 106 Mason James 2012-06-26 13:59:53 UTC Comment hidden (obsolete)
Comment 107 Mason James 2012-06-26 14:03:35 UTC Comment hidden (obsolete)
Comment 108 Mason James 2012-06-26 14:11:12 UTC Comment hidden (obsolete)
Comment 109 Mason James 2012-06-26 14:23:10 UTC Comment hidden (obsolete)
Comment 110 Mason James 2012-06-26 14:29:52 UTC Comment hidden (obsolete)
Comment 111 Mason James 2012-06-26 14:31:44 UTC Comment hidden (obsolete)
Comment 112 Mason James 2012-06-26 14:39:53 UTC Comment hidden (obsolete)
Comment 113 Mason James 2012-06-26 14:42:32 UTC Comment hidden (obsolete)
Comment 114 Mason James 2012-06-26 14:45:26 UTC Comment hidden (obsolete)
Comment 115 Mason James 2012-06-26 14:48:37 UTC Comment hidden (obsolete)
Comment 116 Mason James 2012-06-26 14:52:06 UTC Comment hidden (obsolete)
Comment 117 Mason James 2012-06-26 14:56:10 UTC Comment hidden (obsolete)
Comment 118 Mason James 2012-06-26 20:37:25 UTC Comment hidden (obsolete)
Comment 119 Mason James 2012-06-26 20:37:54 UTC Comment hidden (obsolete)
Comment 120 Mason James 2012-06-26 20:38:06 UTC Comment hidden (obsolete)
Comment 121 Mason James 2012-06-26 20:38:18 UTC Comment hidden (obsolete)
Comment 122 Mason James 2012-06-26 20:38:31 UTC Comment hidden (obsolete)
Comment 123 Mason James 2012-06-26 20:38:46 UTC Comment hidden (obsolete)
Comment 124 Mason James 2012-06-26 20:38:59 UTC Comment hidden (obsolete)
Comment 125 Jonathan Druart 2012-07-18 13:10:13 UTC Comment hidden (obsolete)
Comment 126 Jonathan Druart 2012-07-18 13:12:03 UTC Comment hidden (obsolete)
Comment 127 Jonathan Druart 2012-07-18 13:13:14 UTC Comment hidden (obsolete)
Comment 128 Jonathan Druart 2012-07-18 13:14:10 UTC Comment hidden (obsolete)
Comment 129 Jonathan Druart 2012-07-18 13:49:24 UTC Comment hidden (obsolete)
Comment 130 Jonathan Druart 2012-07-18 13:49:53 UTC Comment hidden (obsolete)
Comment 131 Jonathan Druart 2012-07-18 13:51:58 UTC Comment hidden (obsolete)
Comment 132 Jonathan Druart 2012-07-18 13:52:36 UTC Comment hidden (obsolete)
Comment 133 Jonathan Druart 2012-07-18 13:53:11 UTC Comment hidden (obsolete)
Comment 134 Jonathan Druart 2012-07-18 13:53:44 UTC Comment hidden (obsolete)
Comment 135 Jonathan Druart 2012-07-18 13:54:20 UTC Comment hidden (obsolete)
Comment 136 Jonathan Druart 2012-07-18 13:54:50 UTC Comment hidden (obsolete)
Comment 137 Jonathan Druart 2012-07-18 13:55:24 UTC Comment hidden (obsolete)
Comment 138 Jonathan Druart 2012-07-18 13:55:57 UTC Comment hidden (obsolete)
Comment 139 Jonathan Druart 2012-07-18 13:56:28 UTC Comment hidden (obsolete)
Comment 140 Jonathan Druart 2012-07-18 13:57:02 UTC Comment hidden (obsolete)
Comment 141 Jonathan Druart 2012-07-18 13:57:39 UTC Comment hidden (obsolete)
Comment 142 Jonathan Druart 2012-07-18 13:58:13 UTC Comment hidden (obsolete)
Comment 143 Jonathan Druart 2012-07-18 13:58:43 UTC Comment hidden (obsolete)
Comment 144 Jonathan Druart 2012-07-18 13:59:16 UTC Comment hidden (obsolete)
Comment 145 Jonathan Druart 2012-07-18 14:00:01 UTC Comment hidden (obsolete)
Comment 146 Jonathan Druart 2012-07-18 14:00:38 UTC Comment hidden (obsolete)
Comment 147 Jonathan Druart 2012-07-18 14:01:17 UTC Comment hidden (obsolete)
Comment 148 Jonathan Druart 2012-07-18 14:01:49 UTC Comment hidden (obsolete)
Comment 149 Jonathan Druart 2012-07-18 14:02:49 UTC Comment hidden (obsolete)
Comment 150 Jonathan Druart 2012-07-18 14:10:27 UTC Comment hidden (obsolete)
Comment 151 Jonathan Druart 2012-07-18 14:13:25 UTC Comment hidden (obsolete)
Comment 152 Jonathan Druart 2012-07-18 14:17:19 UTC Comment hidden (obsolete)
Comment 153 Jonathan Druart 2012-07-18 14:21:40 UTC Comment hidden (obsolete)
Comment 154 Jonathan Druart 2012-07-18 14:25:48 UTC
Comment on attachment 10517 [details] [review]
Bug 6679 - [FAILED QA]fix 3 perlcritic violations in C4/ClassSortRoutine.pm

Failed QA :
"Bad name after :: at C4/ClassSortRoutine.pm line 55."
Comment 155 Jonathan Druart 2012-07-18 14:30:48 UTC Comment hidden (obsolete)
Comment 156 Jonathan Druart 2012-07-18 14:32:19 UTC
Comment on attachment 10517 [details] [review]
Bug 6679 - [FAILED QA]fix 3 perlcritic violations in C4/ClassSortRoutine.pm

Failed QA:
Bad name after :: at C4/ClassSortRoutine.pm line 55.
Comment 157 Jonathan Druart 2012-07-18 14:35:11 UTC
All seems good except 2 patchs: C4/Letters.pm and C4/ClassSortRoutine.pm

Switch to Assigned
Comment 158 Colin Campbell 2012-09-14 13:06:37 UTC Comment hidden (obsolete)
Comment 159 Jonathan Druart 2012-09-14 13:30:28 UTC
(In reply to comment #158)
> Created attachment 12224 [details] [review]
> Proposed patch
> 
> This patch fixes two instances that cause Jemkins builds to fail

Why don't you just remove the "undef" word ?
I think
if ($@) {
    return;
}
is less readable as
return if $@;

Don't you think ?
I can provide a new patch but I don't want to go against what you want.
Comment 160 Colin Campbell 2012-09-19 16:30:39 UTC Comment hidden (obsolete)
Comment 161 Paul Poulain 2012-09-19 16:48:17 UTC
Comment on attachment 12363 [details] [review]
Proposed patch to calm Jenkins

This patch has been pushed
Comment 162 Paul Poulain 2012-09-19 16:49:09 UTC
patches are signed off, going to this status to have patches in QA team radar !
Comment 163 Paul Poulain 2012-09-20 09:37:35 UTC
Comment on attachment 10485 [details] [review]
[FAILED-QA]Bug 6679 - fix 19 perlcritic violations from C4::Letters.pm

This patch is now useless, the same goal has been achieved by a follow-up on bug 8063
Comment 164 Paul Poulain 2012-09-20 10:06:35 UTC
Comment on attachment 10494 [details] [review]
Bug 6679 - [SIGNED-OFF]fix 12 perlcritic violations in C4/External/BakerTaylor.pm

This patch has been pushed to master
Comment 165 Paul Poulain 2012-09-20 10:06:53 UTC
Comment on attachment 10495 [details] [review]
Bug 6679 - [SIGNED-OFF] fix 12 perlcritic violations in C4/Auth_with_ldap.pm

This patch has been pushed to master
Comment 166 Paul Poulain 2012-09-20 10:07:07 UTC
Comment on attachment 10496 [details] [review]
Bug 6679 -[SIGNED-OFF] fix 12 perlcritic violations in C4/Context.pm

This patch has been pushed to master
Comment 167 Paul Poulain 2012-09-20 10:07:22 UTC
Comment on attachment 10497 [details] [review]
Bug 6679 - [SIGNED-OFF]fixing 23 '"return" statement with explicit "undef"' violations in C4/Tags.pm

This patch has been pushed to master
Comment 168 Paul Poulain 2012-09-20 10:07:58 UTC
Comment on attachment 10517 [details] [review]
Bug 6679 - [FAILED QA]fix 3 perlcritic violations in C4/ClassSortRoutine.pm

This patch has NOT been pushed.
After applying it, you get a Perl error
Comment 169 Paul Poulain 2012-09-20 10:08:13 UTC
Comment on attachment 10935 [details] [review]
Bug 6679 - [SIGNED-OFF] fix 15 perlcritic violations in C4/ImportExportFramework.pm

This patch has been pushed to master
Comment 170 Paul Poulain 2012-09-20 10:08:41 UTC
Comment on attachment 10936 [details] [review]
Bug 6679 - [SIGNED-OFF] fix 11 perlcritic violations in C4/Dates.pm

This patch has NOT been pushed. After applying it you get a perl error
Comment 171 Paul Poulain 2012-09-20 10:08:56 UTC
Comment on attachment 10938 [details] [review]
Bug 6679 - [SIGNED-OFF] fix 9 perlcritic violations in C4/TmplTokenType.pm

This patch has been pushed to master
Comment 172 Paul Poulain 2012-09-20 10:09:10 UTC
Comment on attachment 10940 [details] [review]
Bug 6679 - [SIGNED-OFF] fix 9 perlcritic violations in C4/Koha.pm

This patch has been pushed to master
Comment 173 Paul Poulain 2012-09-20 10:09:24 UTC
Comment on attachment 10946 [details] [review]
Bug 6679 - [SIGNED-OFF] fix 8 perlcritic violations in C4/Circulation.pm

This patch has been pushed to master
Comment 174 Paul Poulain 2012-09-20 10:09:39 UTC
Comment on attachment 10947 [details] [review]
Bug 6679 - [SIGNED-OFF] fix 8 perlcritic violations in C4/Barcodes.pm

This patch has been pushed to master
Comment 175 Paul Poulain 2012-09-20 10:09:55 UTC
Comment on attachment 10948 [details] [review]
Bug 6679 - [SIGNED-OFF] fix 7 perlcritic violations in C4/Biblio.pm

This patch has been pushed to master
Comment 176 Paul Poulain 2012-09-20 10:10:12 UTC
Comment on attachment 10950 [details] [review]
Bug 6679 - [SIGNED-OFF] fix 6 perlcritic violations in C4/Barcodes/hbyymmincr.pm

This patch has been pushed to master
Comment 177 Paul Poulain 2012-09-20 10:10:26 UTC
Comment on attachment 10952 [details] [review]
Bug 6679 - [SIGNED-OFF] fix 5 perlcritic violations in C4/Serials.pm

This patch has been pushed to master
Comment 178 Paul Poulain 2012-09-20 10:11:02 UTC
Comment on attachment 10953 [details] [review]
Bug 6679 - [SIGNED-OFF] fix 4 perlcritic violations in C4/ShelfBrowser.pm

This patch has been pushed to master
Comment 179 Paul Poulain 2012-09-20 10:11:19 UTC
Comment on attachment 10955 [details] [review]
Bug 6679 - [SIGNED-OFF] fix 4 perlcritic violations in C4/Branch.pm

This patch has been pushed to master
Comment 180 Paul Poulain 2012-09-20 10:11:32 UTC
Comment on attachment 10957 [details] [review]
Bug 6679 - [SIGNED-OFF] fix 3 perlcritic violations in C4/SQLHelper.pm

This patch has been pushed to master
Comment 181 Paul Poulain 2012-09-20 10:11:47 UTC
Comment on attachment 10959 [details] [review]
Bug 6679 - [SIGNED-OFF] fix 3 perlcritic violations in C4/Output.pm

This patch has been pushed to master
Comment 182 Paul Poulain 2012-09-20 10:12:06 UTC
Comment on attachment 10960 [details] [review]
Bug 6679 - [SIGNED-OFF] fix 3 perlcritic violations in C4/Message.pm

This patch has been pushed to master
Comment 183 Paul Poulain 2012-09-20 10:12:22 UTC
Comment on attachment 10961 [details] [review]
Bug 6679 - [SIGNED-OFF] fix 3 perlcritic violations in C4/Items.pm

This patch has been pushed to master
Comment 184 Paul Poulain 2012-09-20 10:12:36 UTC
Comment on attachment 10963 [details] [review]
Bug 6679 - [SIGNED-OFF] fix 2 perlcritic violations in C4/Utils.pm

This patch has been pushed to master
Comment 185 Paul Poulain 2012-09-20 10:12:50 UTC
Comment on attachment 10964 [details] [review]
Bug 6679 - [SIGNED-OFF] fix 2 perlcritic violations in C4/ItemCirculationAlertPreference.pm

This patch has been pushed to master
Comment 186 Paul Poulain 2012-09-20 10:13:19 UTC
Comment on attachment 10965 [details] [review]
Bug 6679 - [SIGNED-OFF] fix 2 perlcritic violations in C4/Installer/PerlModules.pm

This patch has been pushed to master
Comment 187 Paul Poulain 2012-09-20 10:13:44 UTC
Comment on attachment 10966 [details] [review]
Bug 6679 - [SIGNED-OFF] fix 2 perlcritic violations in C4/BackgroundJob.pm

This patch has been pushed to master
Comment 188 Paul Poulain 2012-09-20 10:21:04 UTC
Comment on attachment 10981 [details] [review]
Bug 6679 - [SIGNED-OFF] fix 3 perlcritic violations in C4/Auth.pm, and enabled warnings

This patch has been pushed to master
Comment 189 Paul Poulain 2012-09-20 10:21:19 UTC
Comment on attachment 10980 [details] [review]
Bug 6679 - [SIGNED-OFF] fix 2 perlcritic violations in C4/Print.pm

This patch has been pushed to master
Comment 190 Paul Poulain 2012-09-20 10:21:33 UTC
Comment on attachment 10979 [details] [review]
Bug 6679 - [SIGNED-OFF] fix 4 perlcritic violations in C4/Creators/PDF.pm

This patch has been pushed to master
Comment 191 Paul Poulain 2012-09-20 10:21:49 UTC
Comment on attachment 10976 [details] [review]
Bug 6679 - [SIGNED-OFF] fix 4 perlcritic violations in C4/Installer.pm

This patch has been pushed to master
Comment 192 Paul Poulain 2012-09-20 10:24:04 UTC
Attachment that are still here either:
 * does not apply anymore
 * badly break Koha (resulting in a Perl error)
Comment 193 Paul Poulain 2012-09-20 11:31:58 UTC
Comment on attachment 10938 [details] [review]
Bug 6679 - [SIGNED-OFF] fix 9 perlcritic violations in C4/TmplTokenType.pm

I just reverted this patch, as it break translator (missed to test it when I tested patches..)
Comment 194 Paul Poulain 2012-09-20 12:52:13 UTC
Comment on attachment 10965 [details] [review]
Bug 6679 - [SIGNED-OFF] fix 2 perlcritic violations in C4/Installer/PerlModules.pm

This patch has been reverted = jenkins complain:
perl t/Installer_pm.t 
1..4
ok 1 - use C4::Installer::PerlModules;
ok 2 - The object isa C4::Installer::PerlModules
ok 3 - returns numeric version
not ok 4 - returns correct version
#   Failed test 'returns correct version'
#   at t/Installer_pm.t line 23.
# Looks like you failed 1 test of 4.

After reverting, t/Installer_pm.t is OK

Maybe that's the test that must be upgraded, but until it's done, this patch is reverted
Comment 195 Mason James 2012-09-24 23:11:31 UTC Comment hidden (obsolete)
Comment 196 Mason James 2012-09-24 23:16:48 UTC Comment hidden (obsolete)
Comment 197 Mason James 2012-09-24 23:22:35 UTC Comment hidden (obsolete)
Comment 198 Owen Leonard 2014-07-16 21:57:24 UTC
I see at least a couple of patches on this bug which are not signed off. Should the status be changed to "needs signoff?"
Comment 199 Mason James 2014-12-03 06:34:41 UTC
hmm, this bug has gotten a bit crazy long...

we should start breaking new perl-critic type patches into smaller manageable bugs, else things can get a little confusing
Comment 200 Katrin Fischer 2014-12-08 21:17:44 UTC
Should this be 'needs sign-off' for the last 3 patches?
Can the 'signed off' patches be obsoleted? I am not sure if they have been pushed or not.
Comment 201 Jonathan Druart 2014-12-09 08:52:28 UTC Comment hidden (obsolete)
Comment 202 Jonathan Druart 2014-12-09 08:52:35 UTC Comment hidden (obsolete)
Comment 203 Jonathan Druart 2014-12-09 08:52:41 UTC Comment hidden (obsolete)
Comment 204 Jonathan Druart 2014-12-09 08:52:48 UTC Comment hidden (obsolete)
Comment 205 Jonathan Druart 2014-12-09 08:52:55 UTC Comment hidden (obsolete)
Comment 206 Jonathan Druart 2014-12-09 08:53:03 UTC Comment hidden (obsolete)
Comment 207 Jonathan Druart 2014-12-09 08:53:11 UTC Comment hidden (obsolete)
Comment 208 Jonathan Druart 2014-12-09 08:54:46 UTC
I rebased these patches.
Some have already fixed.
I removed 1:
 Bug 6679 - [SIGNED-OFF] fix 3 perlcritic violations in C4/ClassSortRoutine.pm
It breaks completely Koha.
Comment 209 Mark Tompsett 2015-01-19 06:09:38 UTC
Comment on attachment 34197 [details] [review]
Bug 6679: (follow-up) fix 9 perlcritic violations in C4/TmplTokenType.pm

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

::: C4/TmplTokenType.pm
@@ +90,5 @@
> +sub DECL              { $_decl }
> +sub PI                { $_pi }
> +sub DIRECTIVE         { $_directive }
> +sub COMMENT           { $_comment }
> +sub UNKNOWN           { $_unknown }

DECL, PI, COMMENT, and UNKNOWN seem to have no easy way to test them. Consider improving testing coverage for TmplTokenType explicitly.
Comment 210 Jonathan Druart 2015-01-19 09:33:56 UTC
(In reply to M. Tompsett from comment #209)
> Comment on attachment 34197 [details] [review] [review]
> Bug 6679: (follow-up) fix 9 perlcritic violations in C4/TmplTokenType.pm
> 
> Review of attachment 34197 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: C4/TmplTokenType.pm
> @@ +90,5 @@
> > +sub DECL              { $_decl }
> > +sub PI                { $_pi }
> > +sub DIRECTIVE         { $_directive }
> > +sub COMMENT           { $_comment }
> > +sub UNKNOWN           { $_unknown }
> 
> DECL, PI, COMMENT, and UNKNOWN seem to have no easy way to test them.
> Consider improving testing coverage for TmplTokenType explicitly.

Hello Mark,
The patch did not modify the contain of these subroutines, I don't think tests should be mandatory here.
And I don't have any ideas how I could test them :)
Comment 211 Bernardo Gonzalez Kriegel 2015-05-12 17:30:20 UTC Comment hidden (obsolete)
Comment 212 Bernardo Gonzalez Kriegel 2015-05-12 17:30:35 UTC Comment hidden (obsolete)
Comment 213 Bernardo Gonzalez Kriegel 2015-05-12 17:32:34 UTC Comment hidden (obsolete)
Comment 214 Kyle M Hall 2015-05-22 12:25:54 UTC Comment hidden (obsolete)
Comment 215 Kyle M Hall 2015-05-22 12:26:03 UTC Comment hidden (obsolete)
Comment 216 Kyle M Hall 2015-05-22 12:26:07 UTC Comment hidden (obsolete)
Comment 217 Kyle M Hall 2015-05-22 12:26:11 UTC Comment hidden (obsolete)
Comment 218 Kyle M Hall 2015-05-22 12:26:15 UTC Comment hidden (obsolete)
Comment 219 Kyle M Hall 2015-05-22 12:26:19 UTC Comment hidden (obsolete)
Comment 220 Kyle M Hall 2015-05-22 12:26:23 UTC Comment hidden (obsolete)
Comment 221 Tomás Cohen Arazi 2015-05-26 14:07:40 UTC
Patches pushed to master.

Thanks Mason!
Comment 222 Tomás Cohen Arazi 2015-05-26 19:44:42 UTC
I reverted this patchset. It completely broke Koha.
I'm not sure why it wasn't catched by people who tested it, but my first guess is Perl version, because the semantics for 'eval' might change from one version to another. I haven't checked it on Perl changes.

The page for eval [1] states that this two lines have different semantic meanings:

 - eval $x;     # CASE 1
 - eval { $x }; # CASE 4

which is one of the changes made in several places (notably breaks C4/Installer/PerlDependencies.pm and C4/Dates.pm)

I'll mark it failed QA. My first guess is that we should be either rewriting the hash -> arrayref trick into using anonymous subroutines to do the job, or even refactoring the whole thing (or removing it).

[1] http://perldoc.perl.org/functions/eval.html
Comment 223 Jonathan Druart 2015-05-27 08:20:08 UTC
This has not been reverted.

Tomas,
I would say you could quickly patch with

diff --git a/C4/Dates.pm b/C4/Dates.pm
index 6ed9588..d7be584 100644
--- a/C4/Dates.pm
+++ b/C4/Dates.pm
@@ -105,7 +105,7 @@ sub dmy_map {
     my $xsub    = $dmy_subs{$dformat};
     $debug and print STDERR "xsub: $xsub \n";
     if ( $val =~ /$re/ ) {
-        my $aref = eval {$xsub};
+        my $aref = eval $xsub;

In a first time.

You can blame me because I get the error too (5.20.1), sorry about that.
Comment 224 Jonathan Druart 2015-09-22 11:06:43 UTC Comment hidden (obsolete)
Comment 225 Bernardo Gonzalez Kriegel 2015-10-31 00:26:24 UTC
Sorry, does not apply

Auto-merging C4/Reports/Guided.pm
CONFLICT (content): Merge conflict in C4/Reports/Guided.pm
Comment 226 Mark Tompsett 2016-01-20 19:13:44 UTC
(In reply to Chris Cormack from comment #13)
> All pushed, leaving open for me

Given that all the previous patches were likely pushed, and the tweak to C4::Dates is no longer applicable, should set the status to something other than Patch doesn't apply and perhaps obsolete all of these?
Comment 227 Jonathan Druart 2016-01-21 08:53:41 UTC Comment hidden (obsolete)
Comment 228 Jonathan Druart 2016-01-21 08:53:59 UTC Comment hidden (obsolete)
Comment 229 Jonathan Druart 2016-01-21 08:54:07 UTC Comment hidden (obsolete)
Comment 230 Jonathan Druart 2016-01-21 08:54:19 UTC Comment hidden (obsolete)
Comment 231 Jonathan Druart 2016-01-21 08:54:28 UTC Comment hidden (obsolete)
Comment 232 Jonathan Druart 2016-01-21 08:57:39 UTC
Patches rebased, changes to C4::Dates removed.

Back to needs signoff
Comment 233 Mark Tompsett 2016-01-21 11:48:15 UTC
Comment on attachment 47090 [details] [review]
Bug 6679 - [SIGNED-OFF] fix 3 perlcritic violations in C4/Record.pm

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

::: C4/Record.pm
@@ +380,4 @@
>      }
>  
>      # Preprocessing
> +    eval {$preprocess} if ($preprocess);

post-fixed 'if' logic prevents a higher level of perlcritic, doesn't it?
Comment 234 Jonathan Druart 2016-01-21 15:04:25 UTC
"post-fixed if"
You mean the use of ternary operators?
If yes, please source, I have never heard about that.
Comment 235 Mark Tompsett 2016-01-21 15:20:10 UTC
(In reply to Jonathan Druart from comment #234)
> "post-fixed if"
> You mean the use of ternary operators?
> If yes, please source, I have never heard about that.

http://search.cpan.org/dist/Perl-Critic-1.126/lib/Perl/Critic/Policy/ControlStructures/ProhibitPostfixControls.pm
Comment 236 Jonathan Druart 2016-01-21 15:40:59 UTC
We have a massive usage of ternary operators, it won't violate our perlcritic policy.
Comment 237 Chris Nighswonger 2016-01-21 16:33:10 UTC
FWIW, somewhere in the distant past we had a discussion regarding how strict to enforce Conway-style on our code. As I recall, there was a general consensus at that time to apply only limited requirements. As Jonathan's observation points out: increasing style strictness may require a lot of refactoring.
Comment 238 Mark Tompsett 2016-01-21 19:20:14 UTC
(In reply to Jonathan Druart from comment #236)
> We have a massive usage of ternary operators, it won't violate our
> perlcritic policy.

While true, it's easier to fix it while working on it piece by piece rather than try to refactor massively later. No biggie. It's not something worth failing this over. :)
Comment 239 Mark Tompsett 2016-01-21 23:23:03 UTC
C4::Record unused variables.
Run koha qa test tools -v 2 -c 5.
perlcritic -5 C4/Record.pm
Comment 240 Jonathan Druart 2016-01-22 08:28:06 UTC
Comment on attachment 47090 [details] [review]
Bug 6679 - [SIGNED-OFF] fix 3 perlcritic violations in C4/Record.pm

Let's say we abandone this one...
Comment 241 Mark Tompsett 2016-01-22 13:59:02 UTC
Here is my testing output. It should be noted that patches 1 and 2 are not required for level 5 testing. However, they are necessary for higher levels. It should also be noted that I added:
    [InputOutput::RequireCheckedSyscalls]
    functions = :builtins
    exclude_functions = print
to the perlcriticrc, so I wouldn't have to do stupidity like:
    my $ignore = print "Hello World!";
But that should be a patch to the qa test tools, not Koha itself.

mtompset@debian:~/kohaclone$ git reset --hard origin/master
HEAD is now at 2505a3d Bug 15607: batch patron mod - do not update dates if not                                                                              given
mtompset@debian:~/kohaclone$ perlcritic -5 C4/Installer/PerlModules.pm
C4/Installer/PerlModules.pm source OK
mtompset@debian:~/kohaclone$ perlcritic -5 C4/TmplToken.pm
C4/TmplToken.pm source OK
mtompset@debian:~/kohaclone$ perlcritic -5 C4/TmplTokenType.pm
Subroutine prototypes used at line 86, column 1.  See page 194 of PBP.  (Severit                                                                             y: 5)
Subroutine prototypes used at line 87, column 1.  See page 194 of PBP.  (Severit                                                                             y: 5)
Subroutine prototypes used at line 88, column 1.  See page 194 of PBP.  (Severit                                                                             y: 5)
Subroutine prototypes used at line 89, column 1.  See page 194 of PBP.  (Severit                                                                             y: 5)
Subroutine prototypes used at line 90, column 1.  See page 194 of PBP.  (Severit                                                                             y: 5)
Subroutine prototypes used at line 91, column 1.  See page 194 of PBP.  (Severit                                                                             y: 5)
Subroutine prototypes used at line 92, column 1.  See page 194 of PBP.  (Severit                                                                             y: 5)
Subroutine prototypes used at line 93, column 1.  See page 194 of PBP.  (Severit                                                                             y: 5)
Subroutine prototypes used at line 94, column 1.  See page 194 of PBP.  (Severit                                                                             y: 5)
mtompset@debian:~/kohaclone$ perlcritic -5 opac/ilsdi.pl
Stricture disabled at line 220, column 17.  See page 429 of PBP.  (Severity: 5)
mtompset@debian:~/kohaclone$ git reset --hard origin/master
HEAD is now at 2505a3d Bug 15607: batch patron mod - do not update dates if not given
mtompset@debian:~/kohaclone$ perlcritic -5 C4/Installer/PerlModules.pm
C4/Installer/PerlModules.pm source OK
mtompset@debian:~/kohaclone$ perlcritic -5 C4/TmplToken.pm
C4/TmplToken.pm source OK
mtompset@debian:~/kohaclone$ perlcritic -5 C4/TmplTokenType.pm
Subroutine prototypes used at line 86, column 1.  See page 194 of PBP.  (Severity: 5)
Subroutine prototypes used at line 87, column 1.  See page 194 of PBP.  (Severity: 5)
Subroutine prototypes used at line 88, column 1.  See page 194 of PBP.  (Severity: 5)
Subroutine prototypes used at line 89, column 1.  See page 194 of PBP.  (Severity: 5)
Subroutine prototypes used at line 90, column 1.  See page 194 of PBP.  (Severity: 5)
Subroutine prototypes used at line 91, column 1.  See page 194 of PBP.  (Severity: 5)
Subroutine prototypes used at line 92, column 1.  See page 194 of PBP.  (Severity: 5)
Subroutine prototypes used at line 93, column 1.  See page 194 of PBP.  (Severity: 5)
Subroutine prototypes used at line 94, column 1.  See page 194 of PBP.  (Severity: 5)
mtompset@debian:~/kohaclone$ perlcritic -5 opac/ilsdi.pl
Stricture disabled at line 220, column 17.  See page 429 of PBP.  (Severity: 5)
mtompset@debian:~/kohaclone$ git bz apply 6679
Bug 6679 - Fixing code so it passes basic Perl::Critic tests

47086 - Bug 6679 - [SIGNED-OFF] fix 9 perlcritic violations in C4/TmplTokenType.pm
47087 - Bug 6679 - [SIGNED-OFF] fix 2 perlcritic violations in C4/Installer/PerlModules.pm
47088 - Bug 6679 - Fixing code so it passes basic Perl:::Critic tests
47089 - Bug 6679: (follow-up) fix 9 perlcritic violations in C4/TmplTokenType.pm

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 6679 - [SIGNED-OFF] fix 9 perlcritic violations in C4/TmplTokenType.pm
Applying: Bug 6679 - [SIGNED-OFF] fix 2 perlcritic violations in C4/Installer/PerlModules.pm
Applying: Bug 6679 - Fixing code so it passes basic Perl:::Critic tests
Applying: Bug 6679: (follow-up) fix 9 perlcritic violations in C4/TmplTokenType.pm
mtompset@debian:~/kohaclone$ perlcritic -5 C4/Installer/PerlModules.pm
C4/Installer/PerlModules.pm source OK
mtompset@debian:~/kohaclone$ perlcritic -5 C4/TmplToken.pm
C4/TmplToken.pm source OK
mtompset@debian:~/kohaclone$ perlcritic -5 C4/TmplTokenType.pm
C4/TmplTokenType.pm source OK
mtompset@debian:~/kohaclone$ perlcritic -5 opac/ilsdi.pl
opac/ilsdi.pl source OK
mtompset@debian:~/kohaclone$ ls -la ~/.perlcriticrc
lrwxrwxrwx 1 mtompset mtompset 26 Apr 27  2015 /home/mtompset/.perlcriticrc -> qa-test-tools/perlcriticrc
mtompset@debian:~/kohaclone$ cat ~/qa-test-tools/perlcriticrc
severity = 5
include = ProhibitUnusedVariables
exclude = Miscellanea::RequireRcsKeywords


[Perl::Critic::Policy::BuiltinFunctions::ProhibitStringyEval]
allow_includes =1

[TestingAndDebugging::RequireUseStrict]
equivalent_modules = Modern::Perl

[TestingAndDebugging::RequireUseWarnings]
equivalent_modules = Modern::Perl

[-Modules::RequireBarewordIncludes]

[InputOutput::RequireCheckedSyscalls]
functions = :builtins
exclude_functions = print
mtompset@debian:~/kohaclone$
Comment 242 Mark Tompsett 2016-01-22 13:59:41 UTC Comment hidden (obsolete)
Comment 243 Mark Tompsett 2016-01-22 13:59:48 UTC Comment hidden (obsolete)
Comment 244 Mark Tompsett 2016-01-22 13:59:55 UTC Comment hidden (obsolete)
Comment 245 Mark Tompsett 2016-01-22 14:00:03 UTC Comment hidden (obsolete)
Comment 246 Mark Tompsett 2016-01-22 14:01:38 UTC
I agreed with comment #240 regarding C4/Record.pm
Looks good to me, setting to signed off.
Comment 247 Kyle M Hall 2016-01-22 17:34:01 UTC
Created attachment 47205 [details] [review]
[PASSED QA] Bug 6679 - [SIGNED-OFF] fix 9 perlcritic violations in C4/TmplTokenType.pm

- Subroutine prototypes used at line 86, column 1.  See page 194 of PBP.  (Severity: 5)

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 248 Kyle M Hall 2016-01-22 17:34:12 UTC
Created attachment 47206 [details] [review]
[PASSED QA] Bug 6679 - [SIGNED-OFF] fix 2 perlcritic violations in C4/Installer/PerlModules.pm

Expression form of "eval" at line 71, column 9.  See page 161 of PBP.  (Severity: 5)

Expression form of "eval" at line 85, column 13.  See page 161 of PBP.  (Severity: 5)

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 249 Kyle M Hall 2016-01-22 17:34:16 UTC
Created attachment 47207 [details] [review]
[PASSED QA] Bug 6679 - Fixing code so it passes basic Perl:::Critic tests

opac/ilsdi.pl: Stricture disabled at line 217, column 17.  See page 429 of PBP.  (Severity: 5)

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 250 Kyle M Hall 2016-01-22 17:34:20 UTC
Created attachment 47208 [details] [review]
[PASSED QA] Bug 6679: (follow-up) fix 9 perlcritic violations in C4/TmplTokenType.pm

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 251 Brendan Gallagher 2016-01-27 05:08:34 UTC
Pushed to Master - Should be in the May 2016 release. Thanks!
Comment 252 Julian Maurice 2016-01-27 14:27:29 UTC
Patch pushed to 3.22.x, will be in 3.22.3
Comment 253 Jonathan Druart 2016-01-27 14:52:32 UTC
(In reply to Julian Maurice from comment #252)
> Patch pushed to 3.22.x, will be in 3.22.3

Watch out for bug 15680.
Comment 254 Frédéric Demians 2016-02-12 06:34:25 UTC
This patch has been pushed to 3.20.x, will be in 3.20.9.