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

[DependencyInjection] Enable deprecating parameters#47719

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

Closed

Conversation

@HeahDude
Copy link
Contributor

@HeahDudeHeahDude commentedSep 29, 2022•
edited
Loading

QA
Branch?6.3
Bug fix?no
New feature?yes
Deprecations?no
Tickets~
LicenseMIT
Doc PRTODO

Following#47680, this PR allows deprecating parameters either to move to build parameters, or to rename them or just to be able to remove them in a BC way.

// with ContainerBuilder:$container->setParameter('old_param','xxx');$container->setParameter('new_param','yyy');$container->deprecateParameter('old_param','symfony/xxx','6.2',# Fourth arg is optional, default: 'The parameter "%s" is deprecated.''The param "%s" is deprecated use "%new_param%" instead.');

TODO:

  • Waiting for@nicolas-grekas (and others) to challenge the implementation
  • Update CHANGELOG
  • Add tests

yceruto reacted with rocket emoji
@carsonbotcarsonbot added this to the6.2 milestoneSep 29, 2022
@carsonbotcarsonbot changed the title[DI] [POC] Enable deprecating parameters[DependencyInjection] [POC] Enable deprecating parametersSep 29, 2022
Copy link
Member

@stofstof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This misses the update of the ContainerBuilder merging to copy the deprecations.

HeahDude reacted with thumbs up emoji
@HeahDudeHeahDudeforce-pushed thefeat/deprecate-parameters branch 2 times, most recently from750ef59 to02f5339CompareSeptember 29, 2022 19:38
@HeahDude
Copy link
ContributorAuthor

This misses the update of the ContainerBuilder merging to copy the deprecations.

Thanks for the heads-up :), fixed!

@HeahDudeHeahDudeforce-pushed thefeat/deprecate-parameters branch fromc662cd1 to62455c9CompareOctober 22, 2022 10:30
@HeahDudeHeahDude changed the title[DependencyInjection] [POC] Enable deprecating parameters[DependencyInjection] Enable deprecating parametersOct 22, 2022
@HeahDude
Copy link
ContributorAuthor

The new implementation does not trigger deprecations in the API itself, so the label can be removed.
Thanks for the reviews.

@fabpotfabpot modified the milestones:6.2,6.3Nov 1, 2022
@HeahDudeHeahDudeforce-pushed thefeat/deprecate-parameters branch fromfcb85cd toa22b22cCompareDecember 5, 2022 11:52
@HeahDudeHeahDude requested a review fromstofDecember 5, 2022 11:52
@HeahDudeHeahDudeforce-pushed thefeat/deprecate-parameters branch from8172f78 toe47d070CompareDecember 6, 2022 08:02
@HeahDudeHeahDude requested review fromnicolas-grekas andstof and removed request fornicolas-grekas andstofDecember 6, 2022 13:01
Copy link
Member

@nicolas-grekasnicolas-grekas left a comment•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I just squashed+added a commit with some tweaks.
LGTM thanks!

@HeahDude
Copy link
ContributorAuthor

While trying the feature I realized that theFrozenParameterBag fails to trigger a deprecation due to property visibility inparent::get().
I've added a test that covers this and fixed it by usingprotected, the property remains immutable anyway thanks to the exception indeprecate() (now covered as well).

Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Still 👍 on my side after the last changes
/cc @symfony/mergers any other comments?

@chalasr
Copy link
Member

Thank you@HeahDude.

HeahDude reacted with hooray emoji

@chalasrchalasrforce-pushed thefeat/deprecate-parameters branch fromc806be8 to0f0a2fbCompareJanuary 11, 2023 14:39
@chalasr
Copy link
Member

gh went wrong due to some race condition, merged inbe5fbce

HeahDude reacted with confused emoji

@HeahDudeHeahDude deleted the feat/deprecate-parameters branchJanuary 11, 2023 14:43
@OskarStark
Copy link
Contributor

Looks like there was no doc issue opened 🤔

HeahDude reacted with thumbs up emoji

fabpot added a commit that referenced this pull requestOct 11, 2023
…utput of `debug:container` command (HeahDude)This PR was squashed before being merged into the 6.4 branch.Discussion----------[FrameworkBundle] Add parameters deprecations to the output of `debug:container` command| Q             | A| ------------- | ---| Branch?       | 6.4| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Tickets       | ~| License       | MIT| Doc PR        | ~Since#47719 parameters can be deprecated but one needs to read the deprecation logs carefully.It would be convenient to have the info when dumping them with debug commands.Here's a glimpse of text format (the fixtures in tests can do the rest):<img width="1126" alt="Screenshot 2023-07-18 at 12 50 49 PM" src="https://github.com/symfony/symfony/assets/10107633/6a2ea20b-be3c-4428-bb5d-aa97f3b38803">I don't know if we really want to support all formats since it may break BC somehow if parsers are used to read the output.I still tried to adapt them all in this PR for consistency.But JSON required an object to display both the value and the deprecation, another way could be to add a specific entry for one or all deprecations.Commits-------7963e9d [FrameworkBundle] Add parameters deprecations to the output of `debug:container` command
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@chalasrchalasrchalasr left review comments

@stofstofAwaiting requested review from stof

Assignees

No one assigned

Projects

None yet

Milestone

6.3

Development

Successfully merging this pull request may close these issues.

7 participants

@HeahDude@chalasr@OskarStark@nicolas-grekas@stof@fabpot@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp