The REST api is based on Mojolicious and as such the controllers within this namespace are all based upon `Mojo::Base`, `use Mojo::Base 'Mojolicious::Controller';` to be specific. Mojo::Base enables the following, which are the only useful things achieved by Modern::Perl in our case (as we limit ourselves to supporting perl 5.10, so any and all other features past perl 5.10 are unavailable to use anyway). ``` use strict; use warnings; use utf8; use feature ':5.10'; use mro; use IO::Handle (); require SomeBaseClass; push @ISA, 'SomeBaseClass'; sub has { Mojo::Base::attr(__PACKAGE__, @_) } ``` As such, adding the `use Modern::Perl` statement is just noise and needlessly loads a module.
Modern::Perl will be loaded by other packages anyway :)
Created attachment 84119 [details] [review] Bug 22026: Removed 'use Modern::Perl;' from Koha::REST::classes Test plan: 1) Check all Koha/REST files and confirm that 'use Modern::Perl;' has been removed.
Created attachment 84179 [details] [review] Bug 22026: Removed 'use Modern::Perl;' from Koha::REST::classes Test plan: 1) Check all Koha/REST files and confirm that 'use Modern::Perl;' has been removed. Signed-off-by: David Nind <david@davidnind.com>
Created attachment 84189 [details] [review] Bug 22026: Removed 'use Modern::Perl;' from Koha::REST::classes Test plan: 1) Check all Koha/REST files and confirm that 'use Modern::Perl;' has been removed. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
https://wiki.koha-community.org/wiki/Coding_Guidelines#PERL2:_Modern::Perl should probably be updated to mention this special case
Awesome work all! Pushed to master for 19.05
This cleanup isn't required for 18.11.x.
I am not sure this was a good move, compare what it brings with the changes we need to make. Anyway, now we need to update the coding guidelines and update our t/perlcriticrc (I think). Also Jenkins is failing because of that move, see https://jenkins.koha-community.org/job/Koha_Master_D8/163/ I do not understand why it is only failing for D8.
(In reply to Jonathan Druart from comment #8) > Also Jenkins is failing because of that move, see > https://jenkins.koha-community.org/job/Koha_Master_D8/163/ > I do not understand why it is only failing for D8. % more /etc/debian_version 9.6 % perlcritic --version 1.126 % perlcritic Koha/REST/V1.pm Koha/REST/V1.pm source OK % more /etc/debian_version 8.11 % perlcritic --version 1.122 % perlcritic Koha/REST/V1.pm Code before strictures are enabled at line 37, column 1. See page 429 of PBP. (Severity: 5)
I guess it is: https://github.com/Perl-Critic/Perl-Critic/blob/dev/Changes 1.124 2015-02-27 * The RequireUseStrict and RequireUseWarnings policies now regard Moose, Moo, Mouse, Dancer, Mojolicious, and several other modules as equivalent to the strict and warnings pragma.
Ack, as this would require use packaging a more recent version of perlcritic I don't feel it's a big enough win to warrant the cleanup.. I'm inclined to agree with Jonathan that this point and suggest we revert it until a point in the future.
Reverted from master