-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Use symfony/deprecation-contracts instead of trigger_error #40643
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
Conversation
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.
Awesome. Thank you.
There are a few other places where we use trigger_error
for deprecation.
symfony/src/Symfony/Component/Messenger/Command/AbstractFailedMessagesCommand.php
Line 178 in 5810b6c
@trigger_error(sprintf('The "%s()" method will have a new "string $name" argument in version 5.3, not defining it is deprecated since Symfony 5.2.', __METHOD__), \E_USER_DEPRECATED); symfony/src/Symfony/Component/Mailer/Bridge/Mailgun/Transport/MailgunApiTransport.php
Line 140 in ce8f7e5
@trigger_error(sprintf('Not prefixing the Mailgun header name with "h:" is deprecated since Symfony 5.1. Use header name "%s" instead.', $headerName), \E_USER_DEPRECATED); symfony/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
Line 217 in 5810b6c
@trigger_error('Since symfony/framework-bundle 5.2: Setting the "framework.form.legacy_error_messages" option to "true" is deprecated. It will have no effect as of Symfony 6.0.', \E_USER_DEPRECATED);
Could you verify and update them too?
@Nyholm yes, I will. One of the links relates to |
Yes please |
070f094
to
e361851
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.
Thank you. Just fix this typo
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Messenger/Command/AbstractFailedMessagesCommand.php
Outdated
Show resolved
Hide resolved
74faf7c
to
1d1c145
Compare
Sorry for so many force pushes. It was hard to drop "committed by" from commits. All comments addressed. Thanks for the review. |
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 worries about the many pushes.
Thank you for fixing this.
Btw, Fabbot is a false negative. We should not apply the CS fixes it suggests.
To be merged into 5.2? |
I dont know if this is considered a bugfix or just a minor improvement. Im happy with 5.x and 5.2. |
src/Symfony/Component/Messenger/Command/AbstractFailedMessagesCommand.php
Outdated
Show resolved
Hide resolved
Thank you @andrew-demb. |
…sCommand (nicolas-grekas) This PR was merged into the 5.3-dev branch. Discussion ---------- [Messenger] fix deprecation layer in AbstractFailedMessagesCommand | Q | A | ------------- | --- | Branch? | 5.x | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - Added in #38468, spotted in #40643 Commits ------- 079cb31 [Messenger] fix deprecation layer in AbstractFailedMessagesCommand
This PR was merged into the 5.3 branch. Discussion ---------- [Mailer] Remove deprecation dependency | Q | A | ------------- | --- | Branch? | 5.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - Introduced here (5.3) #40643 But removed was not needed because reverted here (5.2) #41380 Commits ------- dc5c28d Remove deprecation dependency
As
symfony/deprecation-contracts
added to requirements with PR #40468, we can updatetrigger_error
call in favor reusing deprecation abstractions