perl5
perl5 copied to clipboard
Rename Perl version macro
This commit introduces Semantic Versioning macro for Perl version.
The previously named variables: PERL_REVISION, PERL_VERSION, PERL_SUBVERSION are renamed as PERL_VERSION_MAJOR, PERL_VERSION_MINOR, and PERL_VERSION_PATCH
A new variable PERL_VERSION_HEX is also introduced.
References: https://semver.org
@tonycoz I marked this PR as draft, as this need discussion and performed the several changes you requested via cfbc8487e41595e5c9e55b8f6b1e72111f03c0c0
@atoomic, can you provide an update on the status of this p.r. for semantic versioning?
Thank you very much. Jim Keenan
There are several question here:
- Should we add these symbols at all?
- Should they be aliases for PERL_REVISION, PERL_VERSION, and PERL_SUBVERSION; or should it be the other way around.
- That do they really mean. Should they be interpreted as strict semantic versioning, or as the current versioning scheme.
Unless the answer to the first question is no, all of these should be answered (though that answer may depend on the third question).
There are several question here:
1. Should we add these symbols at all? 2. Should they be aliases for PERL_REVISION, PERL_VERSION, and PERL_SUBVERSION; or should it be the other way around. 3. That do they really mean. Should they be interpreted as strict semantic versioning, or as the current versioning scheme.
Unless the answer to the first question is no, all of these should be answered (though that answer may depend on the third question).
No one has responded to @Leont's questions in a year-and-a-half. There's no reason to retain p.r.'s that are in Draft status. I recommend that this p.r. be closed.
I'm pretty sure this change is obsolete, or way, way ahead of its time.
I think the intent was new symbols for a new Perl 7, while maintaining the old names for compatibility with code that only checked PERL_VERSION and PERL_SUBVERSION when checking for compatibility/new features.
I'm not sure it's obsolete. There is still the intention to be able to bump the first component of the version number beyond 5 and into 7, 8, and beyond at some point in the future.
There are many obstacles to doing this, but one of them is the fact that so much existing code on CPAN has a ppport.h
which insists on PERL_REVISION == 5
.
The new macro form at least partly solves this, and does so by renaming the fields to what has been generally accepted industry-wide as a more standard naming structure of MAJOR.MINOR.PATCH
. Overall it's a good idea, and I'm in favour of it.
I'm not sure it's obsolete. There is still the intention to be able to bump the first component of the version number beyond 5 and into 7, 8, and beyond at some point in the future.
There are many obstacles to doing this, but one of them is the fact that so much existing code on CPAN has a
ppport.h
which insists onPERL_REVISION == 5
.The new macro form at least partly solves this, and does so by renaming the fields to what has been generally accepted industry-wide as a more standard naming structure of
MAJOR.MINOR.PATCH
. Overall it's a good idea, and I'm in favour of it.
That makes sense, and answers at least the first two of my questions.
This merge request was created more than two years ago. I reviewed it today. @Leont indicates that 2 of his 3 questions have been answered, but that appears to leave this one:
[What do @atoomic's new or renamed variables] really mean? Should they be interpreted as strict semantic versioning, or as the current versioning scheme?
Could we focus discussion on that?
Also: @atoomic, this p.r. has acquired many merge conflicts. Could you clear them up?
Thank you very much. Jim Keenan
To clarify something, these symbols already exist as aliases to the classic version defines. This PR changes the rest of the code to use the new symbols, and switches the classic defines to be aliases for the new ones.
Even without any change in policy, I think this is a good change to make. I think it's reasonable to refer to the different components of the version as MAJOR, MINOR, and PATCH under the current versioning scheme. And switching everything to use these could allow the old defines to be given "fake" values to assist in bumping the major version past 5.
A policy change related to semver should be considered separately. I doubt we're anywhere close to wanting to make that change. Generally semver is interpreted to also apply to binary compatibility. This would mean bumping the major version each year under our current development process. I don't think this is generally what people want.
I'm definitely keen to see this go in. The new variable names are neater and easier to remember (major.minor.patch is industry-standard, as compared our previous revision.version.subversion - what even is that??!) and also have a certain visual neatness of all being the same length so for that reason alone it's already an improvement.
Additionally it would give us the flexibility to bump the major version up to 7 or beyond at some future date, which is a change we currently cannot make. So keeping that in mind this is also good.
I also feel this is purely a technical-mechanism change. We're not saying "adopt semver" with all the policy suggestions that makes. The code and the commit message don't mention semver, but this PR description currently still does. As long as we all accept that this change itself does not introduce semver I don't think that matters.. but maybe you want to edit the PR description as well to remove that mention if the discussion continues.
But I don't think it has to. The principle seems sound, I have no unanswered questions. If you fix up the conflicts and generally make this merge-ready I'd be happy to give it a review in the hope of merging it and finally putting this one to bed.
The new variable names are neater and easier to remember
I'm not sure that is relevant, given that everyone really ought to be using PERL_VERSION_GE
and PERL_VERSION_LT
anyway.
I can rebase and rework that Pull Request but I will only do that if there is any interest in merging this or part of this Pull Request. If the consensus is that this does not provide value, then let's close the pull request.
On 9/19/22 08:55, ℕicolas ℝ. wrote:
I can rebase and rework that Pull Request but I will only do that if there is any interest in merging this or part of this Pull Request. If the consensus is that this does not provide value, then let's close the pull request.
I would like this done, so +1
— Reply to this email directly, view it on GitHub https://github.com/Perl/perl5/pull/18059#issuecomment-1251137622, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA2DH7W3NUKTOP73LDVSQDV7B5F3ANCNFSM4P454XOQ. You are receiving this because your review was requested.Message ID: @.***>
On Mon, 19 Sept 2022, 22:31 Karl Williamson, @.***> wrote:
On 9/19/22 08:55, ℕicolas ℝ. wrote:
I can rebase and rework that Pull Request but I will only do that if there is any interest in merging this or part of this Pull Request. If the consensus is that this does not provide value, then let's close the pull request.
I would like this done, so +1
Karl, t is not entirely clear what "this" is. I assume you want it merged, but your statement supports both interpretations.
Yves
Yves
On 9/19/22 16:08, Yves Orton wrote:
On Mon, 19 Sept 2022, 22:31 Karl Williamson, @.***> wrote:
On 9/19/22 08:55, ℕicolas ℝ. wrote:
I can rebase and rework that Pull Request but I will only do that if there is any interest in merging this or part of this Pull Request. If the consensus is that this does not provide value, then let's close the pull request.
I would like this done, so +1
Karl, t is not entirely clear what "this" is. I assume you want it merged, but your statement supports both interpretations.
Sorry. I think this PR is a step forward, and should be merged when it is ready to be so.
@atoomic the consensus seems to be that people do want to merge this, but that there are some minor changes required before it merges. Also the PR is currently in conflict. Do you want to get it up to date or should someone else pick it up?
No problems @demerphq I will revisit it an extra time :-) and submit and updated PR
No problems @demerphq I will revisit it an extra time :-) and submit and updated PR
@atoomic, are we going forward with this pull request? (It now has merge conflicts.) Thanks.
It'd be good to get this tidied up and ready to go. Can someone - @atoomic or elsewise - rebase and tidy it up ready to merge?
In the jkeenan/semantic-versioning
branch in the main repository, I have attempted to rebase @atoomic's branch on blead.
https://github.com/Perl/perl5/tree/jkeenan/semantic-versioning
https://github.com/Perl/perl5/compare/jkeenan/semantic-versioning?expand=1
Since it took me 4 tries to get it right, please review it carefully. I'm not sure how or whether I make that code the content of this pull request. Please advise.
It'd be good to get this tidied up and ready to go. Can someone - @atoomic or elsewise - rebase and tidy it up ready to merge?
In the
jkeenan/semantic-versioning
branch in the main repository, I have attempted to rebase @atoomic's branch on blead.https://github.com/Perl/perl5/tree/jkeenan/semantic-versioning
https://github.com/Perl/perl5/compare/jkeenan/semantic-versioning?expand=1
Since it took me 4 tries to get it right, please review it carefully. I'm not sure how or whether I make that code the content of this pull request. Please advise.
@atoomic @leonerd can you please review above? Thanks.
It'd be good to get this tidied up and ready to go. Can someone - @atoomic or elsewise - rebase and tidy it up ready to merge?
In the
jkeenan/semantic-versioning
branch in the main repository, I have attempted to rebase @atoomic's branch on blead.https://github.com/Perl/perl5/tree/jkeenan/semantic-versioning
https://github.com/Perl/perl5/compare/jkeenan/semantic-versioning?expand=1
Since it took me 4 tries to get it right, please review it carefully. I'm not sure how or whether I make that code the content of this pull request. Please advise.
My branch which attempted to rebase @atoomic's original p.r. on blead has now been updated here: https://github.com/Perl/perl5/compare/blead...jkeenan:perl5:semantic-version-upgrade-20240324?expand=1