Bug 26683 - Fix Modules::RequireFilenameMatchesPackage Perl Critic Violations
Summary: Fix Modules::RequireFilenameMatchesPackage Perl Critic Violations
Status: CLOSED INVALID
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: David Cook
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-10-14 23:02 UTC by David Cook
Modified: 2021-06-14 21:28 UTC (History)
1 user (show)

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


Attachments
Bug 26683: Decompose C4::Barcodes::ValueBuilder to make Perl critic happy (9.35 KB, patch)
2020-10-14 23:10 UTC, David Cook
Details | Diff | Splinter Review
Bug 26683: Decompose C4::Barcodes::ValueBuilder to make Perl critic happy (10.74 KB, patch)
2020-10-15 00:14 UTC, David Cook
Details | Diff | Splinter Review
Bug 26683: Decompose Koha::Z3950Responder::RPN into Net::Z3950::RPN::* packages (9.08 KB, patch)
2020-10-15 00:14 UTC, David Cook
Details | Diff | Splinter Review
Bug 26683: Load localized Net::Z3950::RPN::* modules (836 bytes, patch)
2020-10-15 00:52 UTC, David Cook
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2020-10-14 23:02:12 UTC
Both ./C4/Barcodes/ValueBuilder.pm and ./Koha/Z3950Responder/RPN.pm violate the Perl Critic policy of Modules::RequireFilenameMatchesPackage.

You can read more about it at https://metacpan.org/pod/Perl::Critic::Policy::Modules::RequireFilenameMatchesPackage

The failures look like this:

 FAIL   C4/Barcodes/ValueBuilder.pm                                                                      
   FAIL   critic                                                                                         
                # Modules::RequireFilenameMatchesPackage: Got 1 violation(s).

 FAIL   Koha/Z3950Responder/RPN.pm
   FAIL   critic
                # Modules::RequireFilenameMatchesPackage: Got 1 violation(s).
Comment 1 David Cook 2020-10-14 23:10:51 UTC
Created attachment 111657 [details] [review]
Bug 26683: Decompose C4::Barcodes::ValueBuilder to make Perl critic happy
Comment 2 David Cook 2020-10-15 00:14:47 UTC
Created attachment 111666 [details] [review]
Bug 26683: Decompose C4::Barcodes::ValueBuilder to make Perl critic happy
Comment 3 David Cook 2020-10-15 00:14:54 UTC
Created attachment 111667 [details] [review]
Bug 26683: Decompose Koha::Z3950Responder::RPN into Net::Z3950::RPN::* packages

This patch reverts the usage of Net::Z3950::SimpleServer's
samples/render-search.pl example, and instead creates modified packages
which will be loaded before the standard ones due to these being located
higher up in the @INC path.
Comment 4 David Cook 2020-10-15 00:16:11 UTC
C4::Barcodes::ValueBuilder test plan:
1. Iterate through the "autoBarcode" options
2. For each autoBarcode option, try generating a barcode at http://localhost:8081/cgi-bin/koha/cataloguing/additem.pl?biblionumber=29&searchid=scs_1602720764348
Comment 5 David Cook 2020-10-15 00:52:27 UTC
Created attachment 111669 [details] [review]
Bug 26683: Load localized Net::Z3950::RPN::* modules
Comment 6 David Cook 2020-10-15 00:55:38 UTC
Koha::Z3950Responder::RPN test plan:

0. Open 2 terminals

Terminal 1:
1. koha-z3950-responder --stop kohadev
2. perl misc/z3950_responder.pl

Terminal 2:
3. yaz-client 127.0.0.1:2100/biblios
4. #Note connection accepted
5. find @or @1=4 Bruce @1=4 Damian
6. #Note Number of hits: 9
7. #Iterate through results using "show" and notice the 245 has either Bruce or Damian in it

Note that this just makes sure the koha-z3950-responder is still running. It doesn't actually test that the Net::Z3950::RPN::* modules are actually doing anything it seems. 

Ere Maijala might be the best person to verify this, since he's the one who added Koha::Z3950Responder::RPN in Bug 13937 for "Add support for SRU and Elasticsearch"

Before I added the 3rd patch "Load localized Net::Z3950::RPN::* modules", the t/db_dependent/Koha/Z3950Responder/GenericSession.t test was failing, but now it's passing.

--

(It's a shame we don't have a "lib" directory that contains all our Perl modules, but that would require renaming too many files, so I don't see that ever happening...)
Comment 7 David Cook 2020-11-02 22:05:31 UTC
I don't think this is needed thanks to https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=112814