Use symfony/deprecation-contracts instead of trigger_error by andrew-demb · Pull Request #40643 · symfony/symfony

andrew-demb · 2021-03-30T17:14:14Z

Q	A
Branch?	5.x
Bug fix?	no
New feature?	no
Deprecations?	yes
Tickets	-
License	MIT
Doc PR	-

As symfony/deprecation-contracts added to requirements with PR #40468, we can update trigger_error call in favor reusing deprecation abstractions

Nyholm

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?

andrew-demb · 2021-03-31T06:50:25Z

@Nyholm yes, I will.

One of the links relates to symfony/mailer, which doesn't have a dependency on symfony/deprecation-contracts.
Should I add it for this component?

Nyholm · 2021-03-31T06:52:14Z

Yes please

Nyholm

Thank you. Just fix this typo

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

src/Symfony/Component/Messenger/Command/AbstractFailedMessagesCommand.php

andrew-demb · 2021-03-31T07:39:40Z

Sorry for so many force pushes. It was hard to drop "committed by" from commits.

All comments addressed. Thanks for the review.

Nyholm

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.

xabbuh · 2021-03-31T14:44:47Z

To be merged into 5.2?

Nyholm · 2021-03-31T19:54:40Z

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/Translation/PluralizationRules.php

src/Symfony/Component/Messenger/Command/AbstractFailedMessagesCommand.php

nicolas-grekas · 2021-04-01T08:19:50Z

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

carsonbot added Status: Needs Review Deprecation labels Mar 30, 2021

nicolas-grekas approved these changes Mar 30, 2021

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Mar 30, 2021

jderusse approved these changes Mar 30, 2021

View reviewed changes

chalasr approved these changes Mar 30, 2021

View reviewed changes

Nyholm requested changes Mar 31, 2021

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Reviewed labels Mar 31, 2021

andrew-demb requested a review from sroze as a code owner March 31, 2021 07:14

carsonbot added Status: Needs Review and removed Status: Needs Work labels Mar 31, 2021

andrew-demb force-pushed the patch-1 branch 2 times, most recently from 070f094 to e361851 Compare March 31, 2021 07:20

Nyholm reviewed Mar 31, 2021

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php Outdated Show resolved Hide resolved

Nyholm reviewed Mar 31, 2021

View reviewed changes

src/Symfony/Component/Messenger/Command/AbstractFailedMessagesCommand.php Outdated Show resolved Hide resolved

andrew-demb force-pushed the patch-1 branch 2 times, most recently from 74faf7c to 1d1c145 Compare March 31, 2021 07:37

Nyholm approved these changes Mar 31, 2021

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Mar 31, 2021

Nyholm requested a review from chalasr March 31, 2021 07:49

carsonbot added Status: Needs Review and removed Status: Reviewed labels Mar 31, 2021

jderusse reviewed Mar 31, 2021

View reviewed changes

src/Symfony/Component/Translation/PluralizationRules.php Outdated Show resolved Hide resolved

src/Symfony/Component/Messenger/Command/AbstractFailedMessagesCommand.php Outdated Show resolved Hide resolved

derrabus approved these changes Mar 31, 2021

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Mar 31, 2021

nicolas-grekas mentioned this pull request Apr 1, 2021

[Messenger] fix deprecation layer in AbstractFailedMessagesCommand #40667

Merged

Use symfony/deprecation-contracts instead of trigger_error

1acc296

nicolas-grekas force-pushed the patch-1 branch from 1d1c145 to 1acc296 Compare April 1, 2021 08:19

nicolas-grekas merged commit 11db9cd into symfony:5.x Apr 1, 2021

jderusse mentioned this pull request May 23, 2021

[Mailer] Remove deprecation dependency #41389

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Use symfony/deprecation-contracts instead of trigger_error#40643

Use symfony/deprecation-contracts instead of trigger_error#40643
nicolas-grekas merged 1 commit intosymfony:5.xfrom
andrew-demb:patch-1

andrew-demb commented Mar 30, 2021 •

edited

Loading

Uh oh!

Nyholm left a comment •

edited

Loading

Uh oh!

andrew-demb commented Mar 31, 2021

Uh oh!

Nyholm commented Mar 31, 2021

Uh oh!

Nyholm left a comment

Uh oh!

Uh oh!

Uh oh!

andrew-demb commented Mar 31, 2021

Uh oh!

Nyholm left a comment •

edited

Loading

Uh oh!

xabbuh commented Mar 31, 2021

Uh oh!

Nyholm commented Mar 31, 2021

Uh oh!

Uh oh!

Uh oh!

nicolas-grekas commented Apr 1, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Uh oh!

Conversation

andrew-demb commented Mar 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Nyholm left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrew-demb commented Mar 31, 2021

Uh oh!

Nyholm commented Mar 31, 2021

Uh oh!

Nyholm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

andrew-demb commented Mar 31, 2021

Uh oh!

Nyholm left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xabbuh commented Mar 31, 2021

Uh oh!

Nyholm commented Mar 31, 2021

Uh oh!

Uh oh!

Uh oh!

nicolas-grekas commented Apr 1, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

andrew-demb commented Mar 30, 2021 •

edited

Loading

Nyholm left a comment •

edited

Loading

Nyholm left a comment •

edited

Loading