Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Contracts] Rename ServiceSubscriberTrait to ServiceMethodsSubscriberTrait#54496

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

Merged

Conversation

@nicolas-grekas
Copy link
Member

@nicolas-grekasnicolas-grekas commentedApr 5, 2024
edited
Loading

QA
Branch?7.1
Bug fix?no
New feature?no
Deprecations?yes
IssuesFix#54490
LicenseMIT

As described in the linked PR, AbstractController is incompatible with ServiceSubscriberTrait because of the added type to the AbstractController::$container property, while ServiceSubscriberTrait's $container property cannot have a type without a BC break.

There are two parts to this PR:

  • Deprecate ServiceSubscriberTrait in favor if ServiceMethodsSubscriberTrait, which declares the type of the $container property. The new name better conveys its purpose as a bonus.
  • Fix the incompatibility with AbstractController by removing the property declaration on ServiceSubscriberTrait. This means the
    trait will create a dynamic property. Those are deprecated, but since the trait is also deprecated, the upgrade path is clear.

I also tried to improve the description of the trait in the meantime.

/cc@kbond

pableu, kbond, and 7-zete-7 reacted with thumbs up emoji
@carsonbotcarsonbot added this to the7.1 milestoneApr 5, 2024
@nicolas-grekasnicolas-grekas changed the title[Contracts] Rename ServiceSubscriberTrait to ServiceMethodsSubscriber…[Contracts] Rename ServiceSubscriberTrait to ServiceMethodsSubscriberTraitApr 5, 2024
@nicolas-grekasnicolas-grekasforce-pushed theservice-methods-subscriber-trait branch from7f17197 tocaaa488CompareApril 5, 2024 09:30
@nicolas-grekasnicolas-grekasforce-pushed theservice-methods-subscriber-trait branch fromcaaa488 to8f47cedCompareApril 5, 2024 09:39
@fabpot
Copy link
Member

Thank you@nicolas-grekas.

@fabpotfabpot merged commit26d71d3 intosymfony:7.1Apr 5, 2024
@nicolas-grekasnicolas-grekas deleted the service-methods-subscriber-trait branchApril 5, 2024 11:37
@MatTheCat
Copy link
Contributor

This PR seems to break theUnit Tests (8.2, low-deps) job:https://github.com/symfony/symfony/actions/runs/8567827866/job/23480399192#step:8:239

@xabbuh
Copy link
Member

@MatTheCat fixed now after the sub-tree split for the service contracts was synchronised

MatTheCat reacted with thumbs up emoji

javiereguiluz added a commit to symfony/symfony-docs that referenced this pull requestApr 10, 2024
…ated and replaced by `ServiceMethodsSubscriberTrait` (GromNaN)This PR was merged into the 7.1 branch.Discussion----------[DependencyInjection] `ServiceSubscriberTrait` is deprecated and replaced by `ServiceMethodsSubscriberTrait`Fix#19755Related code change:symfony/symfony#54496Commits-------f17331c ServiceSubscriberTrait is deprecated and replaced by ServiceMethodsSubscriberTrait
@fabpotfabpot mentioned this pull requestMay 2, 2024
xabbuh added a commit that referenced this pull requestOct 10, 2024
This PR was merged into the 7.1 branch.Discussion----------[Contracts] add missing properties| Q             | A| ------------- | ---| Branch?       | 7.1| Bug fix?      | no| New feature?  | no| Deprecations? | no| Issues        || License       | MITSince#54496 the property is no longer part of the `ServiceSubscriberTrait` triggering a PHP deprecation like:> Creation of dynamic property Symfony\Contracts\Tests\Service\LegacyTestService::$container is deprecatedCommits-------fbfeefe add missing properties
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@GromNaNGromNaNGromNaN left review comments

@stofstofstof approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

7.1

Development

Successfully merging this pull request may close these issues.

7 participants

@nicolas-grekas@fabpot@MatTheCat@xabbuh@GromNaN@stof@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp