Bug #99310 for Module-CPANTS-Analyse: .pm files should be inspected from blib, not lib, to account for filters

Sat Oct 04 15:38:37 2014 ether [...] cpan.org - Ticket created

Subject:

.pm files should be inspected from blib, not lib, to account for filters

The PM_FILTER option in Makefile.PL can be used to construct .pm files from other sources, or modify their content, before the modules are installed. It is this final content that should be analysed for the various metrics, rather than the raw files shipped in lib/ in the distribution. One example is https://metacpan.org/release/lib-with-preamble -- lib/lib/with/preamble.pm does not contain 'use strict', but 'blib/lib/lib/with/preamble.pm' does. It is the blib/ version that is installed, so it is this version that should be inspected for the metrics. I think MCA should either: - always run 'perl Makefile.PL && make' before inspecting the distribution, or - check Makefile.PL for "PM_FILTER" (I'm not sure what the equivalent Build.PL option is) and run 'perl Makefile.PL && make' if the option is seen.

Sun Oct 05 02:09:00 2014 ishigaki [...] cpan.org - Correspondence added

On Sun Oct 05 04:38:37 2014, ETHER wrote: Show quoted text

> The PM_FILTER option in Makefile.PL can be used to construct .pm files > from other sources, or modify their content, before the modules are > installed. It is this final content that should be analysed for the > various metrics, rather than the raw files shipped in lib/ in the > distribution. > > One example is https://metacpan.org/release/lib-with-preamble -- > lib/lib/with/preamble.pm does not contain 'use strict', but > 'blib/lib/lib/with/preamble.pm' does. It is the blib/ version that is > installed, so it is this version that should be inspected for the > metrics. > > I think MCA should either: > - always run 'perl Makefile.PL && make' before inspecting the > distribution, or > - check Makefile.PL for "PM_FILTER" (I'm not sure what the equivalent > Build.PL option is) and run 'perl Makefile.PL && make' if the option > is seen.

Running Makefile.PL can't (and won't) be an option for MCA because there are distributions whose Makefile.PL runs only on a specific environment (OS, external libraries, etc). Besides, PM_FILTER is not the only option to modify contents of .pm files (PL_FILES, raw perl code in Makefile.PL, or maybe Dist::Zilla plugins can modify code for local sources). It's not reasonable for MCA to take each of those minor options into consideration. As for blib, 1) it conflicts with no_generated_files metric, and 2) MCA needs more refactoring before blib check can be added cleanly. Marked stalled for the latter.

Sun Oct 05 02:09:00 2014 The RT System itself - Status changed from 'new' to 'open'

Sun Oct 05 02:09:01 2014 ishigaki [...] cpan.org - Status changed from 'open' to 'stalled'

Wed Dec 03 03:06:19 2014 REHSACK [...] cpan.org - Correspondence added

The search algorithm should - however - refined: MooX-Cmd sno$ perl -Mblib xt/kwalitee.t ok 1 - meta_json_conforms_to_known_spec ok 2 - meta_json_is_parsable ok 3 - meta_yml_conforms_to_known_spec ok 4 - meta_yml_is_parsable ok 5 - has_buildtool ok 6 - has_changelog ok 7 - has_manifest ok 8 - has_meta_yml ok 9 - has_readme ok 10 - has_tests ok 11 - no_symlinks ok 12 - no_broken_auto_install ok 13 - no_broken_module_install ok 14 - has_human_readable_license ok 15 - has_license_in_source_file ok 16 - has_abstract_in_pod not ok 17 - use_strict # Failed test 'use_strict' # at xt/kwalitee.t line 11. # Error: This distribution does not 'use strict;' (or its equivalents) in all of its modules. Note that this is not about the actual strictness of the modules. It's bad if nobody can tell whether the modules are strictly written or not, without reading the source code of your favorite clever module that actually enforces strictness. In other words, it's bad if someone feels the need to add 'use strict' to your modules. # Details: # The following modules don't use strict (or equivalents): SecondTestApp::Cmd::ifc, ThirdTestApp, ThirdTestApp::Cmd::Foo, SecondTestApp::Cmd::cwo, FailTestApp::Cmd::nothing, FirstTestApp, OptionTestApp::Cmd::Oops, FirstTestApp::Cmd::Test, FirstTestApp::Cmd::Test::Cmd::Test, SecondTestApp # SecondTestApp::Cmd::ifc, ThirdTestApp, ThirdTestApp::Cmd::Foo, SecondTestApp::Cmd::cwo, FailTestApp::Cmd::nothing, FirstTestApp, OptionTestApp::Cmd::Oops, FirstTestApp::Cmd::Test, FirstTestApp::Cmd::Test::Cmd::Test, SecondTestApp # Remedy: Add 'use strict' (or its equivalents) to all modules, or convince us that your favorite module is well-known enough and people can easily see the modules are strictly written. 1..17 # Looks like you failed 1 test of 17. All those affected modules are in t/lib - nothing which is scanned for modules (and most of them are intensionally without "use strict;"). When running Makefile.PL is not an option, scanning blib is an option - or relying on MANIFEST. You can still fallback to current behavior when both better options fail to apply.

Wed Dec 03 03:06:19 2014 The RT System itself - Status changed from 'stalled' to 'open'

Wed Dec 03 03:42:51 2014 ishigaki [...] cpan.org - Correspondence added

Could you tell me the versions of Module::CPANTS::Analyse and Test::Kwalitee you are using, and try the latest versions of both (if not)? On Wed Dec 03 17:06:19 2014, REHSACK wrote: Show quoted text

> The search algorithm should - however - refined: > > MooX-Cmd sno$ perl -Mblib xt/kwalitee.t > ok 1 - meta_json_conforms_to_known_spec > ok 2 - meta_json_is_parsable > ok 3 - meta_yml_conforms_to_known_spec > ok 4 - meta_yml_is_parsable > ok 5 - has_buildtool > ok 6 - has_changelog > ok 7 - has_manifest > ok 8 - has_meta_yml > ok 9 - has_readme > ok 10 - has_tests > ok 11 - no_symlinks > ok 12 - no_broken_auto_install > ok 13 - no_broken_module_install > ok 14 - has_human_readable_license > ok 15 - has_license_in_source_file > ok 16 - has_abstract_in_pod > not ok 17 - use_strict > # Failed test 'use_strict' > # at xt/kwalitee.t line 11. > # Error: This distribution does not 'use strict;' (or its equivalents) > in all of its modules. Note that this is not about the actual > strictness of the modules. It's bad if nobody can tell whether the > modules are strictly written or not, without reading the source code > of your favorite clever module that actually enforces strictness. In > other words, it's bad if someone feels the need to add 'use strict' to > your modules. > # Details: > # The following modules don't use strict (or equivalents): > SecondTestApp::Cmd::ifc, ThirdTestApp, ThirdTestApp::Cmd::Foo, > SecondTestApp::Cmd::cwo, FailTestApp::Cmd::nothing, FirstTestApp, > OptionTestApp::Cmd::Oops, FirstTestApp::Cmd::Test, > FirstTestApp::Cmd::Test::Cmd::Test, SecondTestApp > # SecondTestApp::Cmd::ifc, ThirdTestApp, ThirdTestApp::Cmd::Foo, > SecondTestApp::Cmd::cwo, FailTestApp::Cmd::nothing, FirstTestApp, > OptionTestApp::Cmd::Oops, FirstTestApp::Cmd::Test, > FirstTestApp::Cmd::Test::Cmd::Test, SecondTestApp > # Remedy: Add 'use strict' (or its equivalents) to all modules, or > convince us that your favorite module is well-known enough and people > can easily see the modules are strictly written. > 1..17 > # Looks like you failed 1 test of 17. > > All those affected modules are in t/lib - nothing which is scanned for > modules (and most of them are intensionally without "use strict;"). > > When running Makefile.PL is not an option, scanning blib is an option > - or relying on MANIFEST. You can still fallback to current behavior > when both better options fail to apply.

Mon Dec 08 04:57:31 2014 REHSACK [...] cpan.org - Correspondence added

MooX-Cmd sno$ perl -MModule::CPANTS::Analyse -MTest::Kwalitee= -le 'print Module::CPANTS::Analyse->VERSION; print Test::Kwalitee->VERSION' [...] 0.96 1.22