New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

set perl minimum version to 5.008 #2

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@mstreuhofer

mstreuhofer commented Mar 27, 2015

the required minimum version was set to 5.006 which is not actually
correct since t/20_file/slurp.t depends on "use utf8" which requires
5.008

set perl minimum version to 5.008
the required minimum version was set to 5.006 which is not actually
correct since t/20_file/slurp.t depends on "use utf8" which requires
5.008
@charsbar

This comment has been minimized.

Owner

charsbar commented Mar 27, 2015

Certainly this module requires 5.8 (maybe 5.8.1) for various reasons, but it's not because of "utf8" module (which is available for 5.6, though unreliable. See corelist utf8).

Could you 1) fix your (incorrect) commit message and 2) see also if this actually works with 5.8.0?

@mstreuhofer

This comment has been minimized.

mstreuhofer commented Mar 28, 2015

Perl::MinimumVersion seems to think utf8 is a valid reason to required 5.8 or later.

$ perlver --blame .
 ------------------------------------------------------------
 File    : t/20_file/slurp.t
 Line    : 7
 Char    : 1
 Rule    : _pragma_utf8
 Version : 5.008
 ------------------------------------------------------------
 use utf8;
 ------------------------------------------------------------

You got a commit message being "oops" in that repo, but my commit message is the one which is incorrect and needs a cleanup?

@charsbar

This comment has been minimized.

Owner

charsbar commented Mar 28, 2015

PMV claims "use utf8" is broken under Perl 5.6, and I don't argue that is false, but it's something that SHOULD be avoided, not MUST (it may be unreliable, but at least it can be skipped at run time). Besides, utf8 is not a key component of this module. If utf8 pragma is the only reason to bump up the perl version requirement, it's much better to skip broken tests and keep the requirement as low as possible for better backward compatibility. Thanks to the CPAN Testers, I know previous versions install correctly at least under 5.8.1. If this is confirmed to be installable under 5.8.0, it's nice and worth updating the minimum version (at least from a point of view where you should specify an actual perl minimum version to let toolchain know if the module is installable under a certain environment as soon as possible). If not, updating a bogus version requirement to another bogus requirement is not so useful, because that means there is still ia version that satisfies the minimum requirement but doesn't work properly. That's why I asked you to update the commit message to include a better reason, but ... shrug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment