Some deduplications in parser #23838
Conversation
embed.fnc
Outdated
| p |void |package_v544 |NN OP *o \ | ||
| |NULLOK OP *v |
There was a problem hiding this comment.
This should probably be called "package2()" if you're going for a non-descriptive new name.
Otherwise package_with_version() or package_and_version() seem more reasonable then either.
There was a problem hiding this comment.
oh, that's what I forgot in PR message ... updated.
Honestly, I prefer API version in another version of function - it clearly says when it was added without need to consult additional tools (which in fact are not aware of semantics)
There was a problem hiding this comment.
@tonycoz Perl_package changed, original functions are now static and new function remains package
There was a problem hiding this comment.
Please rename S_package (maybe S_package_named) to keep the base name distinct from Perl_package.
happy-barney
force-pushed
the
hpb/cleanup
branch
from
0c7fed2 to
fe52a7b
Compare
|
Documentation build warnings: |
happy-barney
force-pushed
the
hpb/cleanup
branch
from
1b863e4 to
481a3e2
Compare
|
@tonycoz done |
happy-barney
force-pushed
the
hpb/cleanup
branch
from
481a3e2 to
8715272
Compare
leonerd
left a comment
leonerd
left a comment
There was a problem hiding this comment.
Code looks fine but I'd like to see a slight addition to the docs.
op.c
Outdated
| Function combines former C<package> and C<package_version> into single call. | ||
|
|
There was a problem hiding this comment.
Rather than say "this does what something else that's now been deleted used to do", I'd just explain what this function actually does. Perhaps keep this note as a footnote in the bottom, but it needs explaining in its own terms first.
happy-barney
force-pushed
the
hpb/cleanup
branch
from
8715272 to
f4ec0b6
Compare
op.c
Outdated
| package ($name, $version) | ||
|
|
||
| Function sets current stash name and if not NULL, sets its C<$VERSION>. |
There was a problem hiding this comment.
No need to put the synopsis part; perldoc does that automatically.
Also missing the word "version"; as
... and if I<version> is not NULL, sets its C<$VERSION>.
f4ec0b6 to
916af43
Compare
|
Your tests are all currently failing, due to the mismatch in parameter names between You probably want to go fix the names in |
happy-barney
force-pushed
the
hpb/cleanup
branch
from
916af43 to
9931558
Compare
op.c
Outdated
| /* | ||
| =for apidoc package | ||
|
|
||
| package ($name, $version) |
There was a problem hiding this comment.
This line isn't necessary. The perldoc generator will create that sort of thing for you.
Function combines call of original `package` and `package_version` when new namespace statement is detected. Instead of required three statements usage now consists of single function call.
happy-barney
force-pushed
the
hpb/cleanup
branch
from
9931558 to
b03c9cd
Compare
3 participants
Internal changes - move duplicated code into dedicated functions.