Use Perl::Critic::Freenode to determine good coding style

[kraih]

This should be a well balanced set of rules to encourage a slightly better coding style. I've also fixed the dependency on JSON, which was missing from cpanfile, and removed the dependency on List::MoreUtils, which was unnecessary.

Progress: https://progress.opensuse.org/issues/44597.

[kraih]

Severity 4 (stern) is stricter than before, but still not very strict. You can check manually with perlcritic -1 lib to see all rules applied. I do suggest following all rules (and will do so myself), but enforcing them seems unrealistic for now. Maybe we can lower severity over time (next step would be 3 then).

[codecov]

Codecov Report

Merging #1902 into master will decrease coverage by 0.08%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1902      +/-   ##
==========================================
- Coverage   90.56%   90.47%   -0.09%     
==========================================
  Files         148      148              
  Lines       10266    10369     +103     
==========================================
+ Hits         9297     9381      +84     
- Misses        969      988      +19

Impacted Files	Coverage Δ
lib/OpenQA/Worker/Cache.pm	93.56% <ø> (-0.03%)	⬇️
lib/OpenQA/Schema/Result/Workers.pm	96.77% <100%> (ø)	⬆️
lib/OpenQA/WebAPI/Controller/LiveViewHandler.pm	92.2% <100%> (-3.88%)	⬇️
lib/OpenQA/Client/Archive.pm	81.66% <100%> (ø)	⬆️
lib/OpenQA.pm	69.23% <0%> (-2.57%)	⬇️
lib/OpenQA/Worker/Engines/isotovideo.pm	96.08% <0%> (-0.56%)	⬇️
lib/OpenQA/Utils.pm	91.92% <0%> (-0.19%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8d6b8d6...6082894. Read the comment docs.

[Martchus]

You need to add perl(Perl::Critic::Freenode) to t_requires in openQA.spec, too.

[kraih]

Also forgot to link to the actual rules that Perl::Critic::Freenode uses. They do mostly overlap with the high severity rules from core perlcritic, but some are not included, like BuiltinFunctions::ProhibitStringyEval (which has many valid uses and needs to be judged on a case by case basis).

[Martchus]

Despite WIP tag it looks already good to merge.

[kraih]

@Martchus Haha, and just as you say that the random cache service bug you were hunting hits.

    #   Failed test 'job enqueued'
    #   at ./t/25-cache-service.t line 442.
    # Looks like you failed 1 test of 2.
#   Failed test 'Test Minion task registration and execution'
#   at ./t/25-cache-service.t line 448.
Can't call method "execute" on an undefined value at ./t/25-cache-service.t line 443.

Not restarting the Travis job yet in case you want to look at the result first.

[Martchus]

Good, I'll have a look.