-
Notifications
You must be signed in to change notification settings - Fork 571
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
modernize mkppport and lib/unicore/mktables #17638
Conversation
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.
Why do you have to reproclaim strict, etc in every package?
sub set_fate { | ||
my $self = shift; | ||
my $fate = shift; | ||
my $reason = shift; # Ignored unless suppressing |
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.
$reason is not optional, and the comment should be retained
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.
comment restored
nothing was enforcing $reason
to be a mandatory argument, and it was possible before this change to call it without a reason, I can introduce a behavior change and make it mandatory
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.
Too few arguments for subroutine 'Property::set_fate' at lib/unicore/mktables line 9176.
in sub set_proxy_for
is calling it with a single arg
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.
restoring it as an optional argument for now
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 thought I'd checked during my review that it was always present, but if not, ok
Also regenerate files depending on lib/unicore/mktables ./perl -Ilib regen/mk_invlists.pl; ./perl -Ilib regen/regcharclass.pl
@khwilliamson fbdf3dd is providing the changes requested during the first review, let smoke it before merging changes can probably be spotted from 1e192bb [ which I had to modify later to fix mktable ] |
use function signatures in lib/unicore/mktables and mkppport avoiding some boilerplates to check for @_.