Closed leonerd closed 1 year ago
This looks great to me. @haarg do you have any concerns?
I don't have any objections to this being released now, although it won't make it into 5.32.0 due to the code freeze.
This isn't the first module of this type. MooseX::Declare, Moops, Dios, and Zydeco for example. Which of them do we want to support? If this was only concerning Module::Metadata, I would probably be fine with adding whatever. But supporting this properly will involve changes elsewhere in the ecosystem.
Adding this to Module::Metadata alone won't allow this to work across toolchain uses. In particular, ExtUtils::MakeMaker and Parse::PMFile are used in a number of places, and various installers etc won't work if they aren't updated. PAUSE also has a package parser, although it can be bypassed with provides
metadata, so it isn't particularly important. We don't need to have patches written, but I'd like to at least decide how we want to handle this in other places. We already have had problems because Module::Metadata's parsing doesn't match what the other implementations do.
This is the sort of thing we could raise at the next PTS (which might be virtual) to come to a Consensus Decision on.
Why use the /o
flag in:
elsif ( $line =~ /$CLASS_REGEXP/o ) {
Since as perlre
states:
o - pretend to optimize your code, but actually introduce bugs
🤔
Module::Metadata 1.000038 now recognizes class
, since it is part of core. thanks!
Currently the extra syntax is guarded by requiring a line looking like
in order not to get confused by any other syntax as possibly provided by other modules. The eventual goal of
Object::Pad
andCor
is that hopefully one day this will become core syntax, at which point such guarding can be removed.