-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Add void (PHPdoc) return types #49347
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
Add void (PHPdoc) return types #49347
Conversation
The reason the However, I've been working with other codebases that used the 👍🏻 from my side. I'll see if I can review the PR soon, but I'll probably do so in multiple sessions because my head will start to spin if I try to review it in one go. 🙈 |
The formatter/handler/processor classes from the Monolog bridge are flagged |
d606983
to
73c34f4
Compare
I think our policy is "when flagged with |
73c34f4
to
e426cf2
Compare
No, that's the case for classes that we annotate with With |
e426cf2
to
5eef2f4
Compare
You might be saying the same thing :) |
To elaborate a bit more on the For classes flagged as That being said, we probably should be less reluctant with real |
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.
First batch. I'm through with bridges and bundles. 😓
src/Symfony/Bridge/Monolog/Processor/ConsoleCommandProcessor.php
Outdated
Show resolved
Hide resolved
src/Symfony/Bundle/SecurityBundle/DataCollector/SecurityDataCollector.php
Show resolved
Hide resolved
src/Symfony/Bundle/WebProfilerBundle/EventListener/WebDebugToolbarListener.php
Show resolved
Hide resolved
In case you missed it, the patch failure in the 8.1 tests is fixed when #49346 is merged (and merged up into 6.3). |
1291a58
to
ab35772
Compare
caf707d
to
cd171f3
Compare
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.
Next batch, components Asset - Form.
47eac97
to
f1c95be
Compare
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 pushed some changes (see "force-pushed" links in the Github UI).
All green, ready on my side. 👍
We review and patch methods on interfaces (like |
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.
Next batch. I've reviewed all components until Routing + changes done after my last batch.
src/Symfony/Bundle/FrameworkBundle/Command/ContainerDebugCommand.php
Outdated
Show resolved
Hide resolved
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.
Last batch. I need a break now. 🤯
f1c95be
to
cb352f9
Compare
cb352f9
to
508ed81
Compare
Yep, that's what I suggested in #47551 (comment) |
Thank you @wouterj. |
Thank you Nicolas for bringing these raw PRs in good shape in a 4 hour timeframe :) |
And thanks to Alexander as well for spending a morning looking at |
This introduces lots of void return types to the codebase, following our policy:
@return void
to any methods that return void is OKvoid
PHP return type is only OK if the method is internal, private or final (excluding@final
).The changes in this PR are automated through a modified Psalter script. We should particularly review if
@final
classes/methods didn't get real PHP void return types, as this is a BC break.