Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
add parameter type declarations to private methods#32786
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
add parameter type declarations to private methods#32786
Uh oh!
There was an error while loading.Please reload this page.
Conversation
xabbuh commentedJul 27, 2019
| Q | A |
|---|---|
| Branch? | 4.4 |
| Bug fix? | no |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | |
| License | MIT |
| Doc PR |
6c01625 to6462261CompareUh oh!
There was an error while loading.Please reload this page.
c35772b to2d7d64bComparexabbuh commentedJul 30, 2019
Status: Needs review |
nicolas-grekas 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.
deps=low line on Travis consistently fails with a timeout, this looks related, isn't it?
99487df to980899eCompare84443ec to130c4a2Compare130c4a2 to1b2aaa4Comparenicolas-grekas 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.
(we investigated that the failure is a pure CI artefact and should be fixed after merge)
Tobion commentedAug 1, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I think it's good to do that here. But also I think it's a huge risk to add incompatibilities that are not covered by tests. I would not trust the phpdocs all the time and private functions mostly had no phpdocs at all. So the only way the add types there is by tracing all the entry points individually which I guess was a huge pain and is almost impossible to review. |
fabpot commentedAug 1, 2019
As this is for 4.4, we will have plenty of people testing this in real life, which is way much better than any static analysis tool. |
fabpot commentedAug 1, 2019
Thank you@xabbuh. |
This PR was merged into the 4.4 branch.Discussion----------add parameter type declarations to private methods| Q | A| ------------- | ---| Branch? | 4.4| Bug fix? | no| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets || License | MIT| Doc PR |Commits-------1b2aaa4 add parameter type declarations to private methods
This PR was merged into the 4.4 branch.Discussion----------remove some more useless phpdocs| Q | A| ------------- | ---| Branch? | 4.4| Bug fix? | no| New feature? | no <!-- please update src/**/CHANGELOG.md files -->| BC breaks? | no <!-- seehttps://symfony.com/bc -->| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tests pass? | yes <!-- please add some, will be required by reviewers -->| Fixed tickets | #... <!-- #-prefixed issue number(s), if any -->| License | MIT| Doc PR | symfony/symfony-docs#... <!-- required for new features -->Fix some leftovers from#32974 and#32786Commits-------9be4d17 remove some more useless phpdocs
| * | ||
| * @param string $sessionId Session ID | ||
| * @param string $sessionData Encoded session data | ||
| * @param int $maxlifetime session.gc_maxlifetime |
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.
quite a lot of useful phpdoc descriptions have been removed :/
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.
Which of those would you like to keep?
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.
maxlifetime
…s (xabbuh)This PR was merged into the 4.4 branch.Discussion----------add parameter type declarations to private methods| Q | A| ------------- | ---| Branch? | 4.4| Bug fix? | no| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets || License | MIT| Doc PR |Commits-------1b2aaa4 add parameter type declarations to private methods