Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[HackDay][Messanger]Improve PHP DocBlock for Messanger Stamps#29529
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
ogizanagi commentedDec 8, 2018 • 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.
Thanks for taking the time to prepare and suggest these changes@Gramzivi. But actually we try to avoid adding "useless" phpdoc and you'll find a bunch of PRs in this repository trying to remove any of these unnecessary docblocks (see#25859 for instance). By "useless" here, I mean:
I hope you'll understand and keep trying to contribute :) |
| private$result; | ||
| /** | ||
| * @var string |
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 don't add@var phpdoc on properties in Symfony, especially when the constructor allows IDEs to infer it already.
| } | ||
| /** | ||
| * @return string |
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.
Please remove this phpdoc. We don't add it when it provides no value compared to the signature. Here, it only duplicates 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.
actually, this applies to most places changed in that PR.
ro0NL commentedDec 8, 2018
https://symfony.com/doc/current/contributing/code/standards.html#documentation
We should start clarifying this :) and perhaps add a mention in the PR template even. |
nicolas-grekas commentedDec 8, 2018
Closing as explained, thanks for submitting. |
Add PHPDoc missing params for Messanger Stamps