-
Notifications
You must be signed in to change notification settings - Fork 8k
ext/reflection: Add ReflectionConstant::inNamespace() method #20902
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
base: master
Are you sure you want to change the base?
Conversation
iluuu1994
left a comment
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.
LGTM, thanks @khaledalam! There were some recent policy changes:
https://github.com/php/policies/blob/main/release-process.rst#feature-selection-and-development
A core developer MAY also request that the feature be discussed on the internals mailing list, in which case an additional two-week period MUST pass without objection or RFC request before the feature can be merged.
Trivial new functions should briefly be discussed on the internals mailing list. If nobody objects after two weeks, this should be ok to merge.
|
Correction, I missed this part:
So I suppose no discussion is needed. (I don't object to this function). So @DanielEScherzer, please merge if you see appropriate. |
TimWolla
left a comment
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 agree this is an obvious addition that follows the existing design and thus there is nothing to discuss about.
I interpreted this to mean that the PR needs to be open for a month to allow someone to raise an objection or request an RFC, i.e. that this shouldn't be merged until February 10 |
DanielEScherzer
left a comment
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 support this, but marking as "request changes" to make sure it doesn't get merged before February 10
Description
Fix #19557
This PR adds the missing
inNamespace()method toReflectionConstantto achieve full parity withReflectionClassandReflectionFunctionAbstractfor API consistency.Background
ReflectionConstantalready has:getShortName()- returns the short name without namespacegetNamespaceName()- returns the namespace nameBut was missing:
inNamespace()- returns boolean indicating if in a namespace