Perl-Critic / Test-Perl-Critic

Run Perl::Critic as a unit test
http://perlcritic.com
Other
9 stars 7 forks source link

Please make the MCE dependency optional #1

Open karenetheridge opened 9 years ago

karenetheridge commented 9 years ago

Would it be possible to add the MCE prereq to 'recommends' instead of 'requires', and only make use of it when installed? This is to prevent dependency bloat on systems where it's not wanted.

thaljef commented 9 years ago

As I recall, the toolchain does not install recommended modules by default. I would prefer to have an opt-out instead of opt-in. So MCE would be required, unless you explicitly say you don't want it.

Would that be acceptable?

karenetheridge commented 9 years ago

So MCE would be required, unless you explicitly say you don't want it.

I guess that depends on how you propose to implement that.

karenetheridge commented 9 years ago

PS CPAN.pm does default to installing recommendations.

thaljef commented 9 years ago

I guess that depends on how you propose to implement that.

Probably as a Build.PL command line option, with an environment variable hook. Other suggestions?

CPAN.pm does default to installing recommendations.

I didn't know that. Thanks for educating me. But cpanm still does not.

karenetheridge commented 9 years ago

This is the canonical way to handle optional features with prompts:

in Makefile.PL:

my %WriteMakefileArgs = (
   ...
);

WriteMakefileArgs{PREREQ_PM}{'MCE'} = '0'
  if $ENV{PERL_MM_USE_DEFAULT} or prompt("install MCE (to allow parallelized tests)? [Y/n]", 'Y') =~ /^y/i;

WriteMakefileArgs{META_MERGE}{optional_features}{'parallel_testing'} = {
  description => 'parallel tests',
  prereqs => { runtime => { requires => { 'MCE' => '0' } } },
  x_default => 1,
};

WriteMakefile(%WriteMakefileArgs);

This will add the extra prereq unless the user specifically said 'no' to the prompt (and in non-interactive installs, it defaults to 'yes').

I don't know how to do meta merging in Module::Build, but I wouldn't ever recommend using MB anyway unless you really needed to (e.g. in an Alien dist).

thaljef commented 9 years ago

This is the canonical way to handle optional features with prompts:

Thanks for that.

MB was a decision we made long ago, when MB was more fashionable. I could probably switch Test::Perl::Critic to EU::MM in the near future, but Perl::Critic itself will be harder to migrate.

karenetheridge commented 9 years ago

we can help on #toolchain :)

marioroy commented 7 years ago

++ for adding the MCE prereq to 'recommends' instead of 'requires'

karenetheridge commented 7 years ago

Hi all. I never knew that moving older MCE releases to backPan causes this. That is not safe to do. Going forward, will not move MCE releases to backPan. :(

uh, what? moving things to backpan is recommended, to free up space on mirrors, and should not be causing tooling issues.

On Tue, Mar 7, 2017 at 3:13 AM, Mario Roy notifications@github.com wrote:

Hi all. I never knew that moving older MCE releases to backPan causes this. That is not safe to do. Going forward, will not move MCE releases to backPan. :(

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Perl-Critic/Test-Perl-Critic/issues/1#issuecomment-284693083, or mute the thread https://github.com/notifications/unsubscribe-auth/AASfyy6GPE0yZZx1lpOWCxcwzwV2VF0Yks5rjTv1gaJpZM4DbIqw .

marioroy commented 7 years ago

Thank you. I had mistakenly mis-diagnosed an email reply on issue #707.

petdance commented 7 years ago

I'm certainly in favor of moving back to vanilla Makemaker. If I do that, I'll make MCE optional.

iynehz commented 6 years ago

++