-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
Added retrieval of sections #10851
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
Added retrieval of sections #10851
Conversation
umpirsky
commented
May 3, 2014
Q | A |
---|---|
Bug fix? | no |
New feature? | yes |
BC breaks? | no |
Deprecations? | no |
Tests pass? | yes |
Fixed tickets | #10145 |
License | MIT |
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 think in this case Section class should be extracted into the separate file. Also @internal
phpDoc should be removed from this class annotation.
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.
Yes, I agree. Will do.
Done. |
Any reason not to merge this? |
@umpirsky yes. 2.5 reached its feature freeze 1.5 month ago so no new features are merged in it anymore. and merging 2.6 PRs has not started yet |
@stof So basically this PR just waits for 2.6? |
Thank you @umpirsky. |
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.
Missing phpDoc here, at least the return type
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.
There is phpdoc on the class attribute so return type can be figured out by any IDE if that is the point.
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 was just wondering if it is consistent with the actual code base?
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.
There is no general rule. I am not big fan of phpdocs, unless you have something really useful to say in them. 😄
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.
but here, you have something to say: @return Section[]
to document the return type
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.
Isn't it obvious?
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.
no it is not. Your IDE cannot guess that the return value is an array of Section objects when providing completion (well, some IDE could because we have phpdoc on the private property here, but we generally prefer documenting public stuff). The API doc builder cannot guess it either.
And someone not reading the full code may not guess it either.
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.
Fixed in #11067.
…) (umpirsky) This PR was merged into the 2.6-dev branch. Discussion ---------- [Stopwatch] Added return type to Stopwatch::getSections() #10851 (comment) Commits ------- ab00361 Added return type to Stopwatch::getSections()