-
Notifications
You must be signed in to change notification settings - Fork 602
RMG: Simplify the corelist section #23401
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
RMG: Simplify the corelist section #23401
Conversation
|
I can't really comment. The wording I added to the RMG 12 years ago just reflected that at the time, handling Module::CoreList was complex as it lived in all branches and needed to be consistently version-bumped. I can't remember the details, and I have no idea what (if anything) has changed or improved in the meantime. |
Porting/release_managers_guide.pod
Outdated
| $ ./perl -Ilib Porting/corelist.pl ~/my-cpan-mirror | ||
|
|
||
| Otherwise, run: | ||
| In most cases, run: |
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.
I don't like this wording very much.
To me the "in most cases" is meaningless because when reading that I will start wondering if my current case is part of the "most cases" or if it's one of the exception cases..
(From just the diff it's not clear if/when "most cases" apply, maybe it's from the rest of the context but I didn't read that yet)
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.
I fixed it by specifying which type of releases are actually concerned
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.
That looks better.
The only thing that still bothers me slightly is that it's now unclear what needs to happen for MAINT releases..
While I get that the old section is confusing and annoying and cryptic it might still be better than nothing.. :/
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.
OK, I get the point and you're right. I reinstalled the useful bits from the old section :)
|
@bingos would you like to comment on this pull request re |
96af425 to
2569225
Compare
@iabyn I don't think anything changed/improved on this side, my proposed change is really about the form and making the paragraph shorter, reordered (with most common case first) and more "accessible" |
2569225 to
b2d718a
Compare
|
I updated again and I think I got it right. The section keeps the useful bits related to MAINT from the old section, better links to the email, reorders to make most common case to appear first, and makes some very little changes on the wording. |
b2d718a to
74a2815
Compare
74a2815 to
10daac8
Compare
I am unhappy with the "Update Module::CoreList" sections.
Changes