-
Notifications
You must be signed in to change notification settings - Fork 8
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
[preview] Pull Request to bump blead to 7.1.0 #181
base: blead
Are you sure you want to change the base?
Conversation
fa6b0cf
to
77b11c8
Compare
cf6e07a
to
89096dd
Compare
2d608be
to
d4575b2
Compare
@atoomic, This is a p.r. from one branch in your repository to branch Thank you very much. |
@atoomic, @toddr : If this p.r. ultimately makes its way to Perl 5 blead, does it in any way affect how I and (hopefully) others will work in pursuit of the objectives on our roadmap, the next of which is Objective 2? Thank you very much. |
@jkeenan blead in this repo points to the state blead was in Perl5 repo a few years ago |
@jkeenan this PR once merged will not impact the work you are doing for alpha. |
api_revision='7' | ||
api_subversion='0' | ||
api_version='1' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any reason we're keeping these at their "old" names, rather than renaming them to api_version_{major,minor,patch}
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Backwards compatibility. This looks like exactly the thing Perl-Toolchain-Gang/ExtUtils-MakeMaker#358 needed. Generally speaking we can't remove %Config
keys.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I would like to tackle these as a following task: patchlevel, subversion, api_*... macros are legacy
I think @ether is right, we want to remove them from core and let Devel-PPPort provide the backward compatibility layer. But this seems to me a follow up task and should not be part of this initial bump to 7.1.0
.
for ('default', sort grep /\.\d[02468]/, keys %feature_bundle) { | ||
$::bundle = ":$_"; | ||
$::feature = join ' ', @{$feature_bundle{$_}}; | ||
foreach my $bundle ('default', sort grep /\.\d*[02468]\z/a, keys %feature_bundle) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The \z
in here looks wrong. It will skip the "5.9.5" three-component one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are right about the \z
but removing it will have no effects
note that 5.9.5
bundle is currently not listed in feature.pm
view https://metacpan.org/pod/feature#FEATURE-BUNDLES
are you saying that this is a bug in blead and we should fix it?
Do you still think \z
should be removed? I'm good with both options
Did read. Apart from a few things already mentioned, I had only a whitespace nit or two, so: 👍🏽 |
28690e2
to
0431157
Compare
On 8/2/20 2:14 AM, H.Merijn Brand wrote:
***@***.**** commented on this pull request.
------------------------------------------------------------------------
In dist/Devel-PPPort/t/ppphtest.t
<#181 (comment)>:
> @@ -721,7 +721,7 @@ my %p;
my $fail = 0;
for ***@***.***) {
my($name, $flags) = /^(\w+)(?:\s+\[(\w+(?:,\s+\w+)*)\])?$/ or $fail++;
- exists $p{$name} and $fail++;
+ { exists $p{$name} and $fail++; }
As a single change, this looks weird and confusing, certainly because
the surrounding lines do not use an extra scope for the same construct.
As Devel::PPPort is hard enough to maintain, I'd indeed like to see the
code as non-complex as possible and drop the extra scope, but I would
not consider it a blocker.
—
I'll have another PR for D:P today
|
e0f2e72
to
fd89fe2
Compare
failures noticed on windows
|
334c9a4
to
1114834
Compare
1114834
to
c7f82f1
Compare
Use new core PERL_VERSION compare macros in vxs.inc When bumping version which this change we would need to make sure to use the last version of ppport.h
To provide backward compatibility with XS code PERL_REVISION, PERL_VERSION and PERL_SUBVERSION are frozen at 5.255.255 - view patchlevel.h This is introducing new Perl semantic versioning macros: PERL_VERSION_MAJOR, PERL_VERSION_MINOR, PERL_VERSION_PATCH It's recommended to use the compare functions PERL_VERSION_EQ, PERL_VERSION_LT... instead of a direct access to the `VERSION` macros.
c7f82f1
to
b927ee9
Compare
This pull request bumps the Perl version to 7.1.0 with some incremental changes:
Note that in order to provide backward compatibility with XS distributions, PERL_REVISION, PERL_VERSION and PERL_SUBVERSION are frozen at
5.255.255
(view patchlevel.h)We are now instead using some semantic versioning macros: PERL_VERSION_MAJOR, PERL_VERSION_MINOR, PERL_VERSION_PATCH
It's recommended to use the compare functions PERL_VERSION_EQ, PERL_VERSION_LT... instead of a direct access to these macros.