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

[OptionsResolver] Trigger deprecation only if the option is used#28878

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
fabpot merged 1 commit intosymfony:masterfromyceruto:trigger_deprecation
Oct 24, 2018

Conversation

@yceruto
Copy link
Member

@ycerutoyceruto commentedOct 15, 2018
edited
Loading

QA
Branch?master
Bug fix?no
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets#28848
LicenseMIT
Doc PRsymfony/symfony-docs#10496

It's true that showing a deprecation message when the option is not used is a bit annoying and it would be heavy to get rid of it.

Now, a deprecated option is triggered only when it's provided by the user or each time is being called from a lazy evaluation (except for deprecations based on the value, they're triggered only when provided by the user).

@yceruto
Copy link
MemberAuthor

yceruto commentedOct 15, 2018
edited
Loading

There is still a desicion to make here to complete this patch:

#28848 (comment) ... (but there should be a way to opt-out of the deprecation to be able to write a BC layer)

#28860 (comment)offsetGet($option/*, bool $triggerDeprecation = true*/) seems fine? That be the easiest :) It's not needed yet, but surely sooner or later, for lib owners a must (so we should ship it as a feature)

See example inhttps://github.com/symfony/symfony/pull/28860/files#diff-c6198934d4f35bc102ff362d40b71285R40

WDYT about the@ro0NL's idea? thoughts?

@ycerutoycerutoforce-pushed thetrigger_deprecation branch 3 times, most recently fromd57f826 to06ca456CompareOctober 16, 2018 12:17
@ycerutoycerutoforce-pushed thetrigger_deprecation branch 2 times, most recently fromaa247e2 todb8a3dbCompareOctober 16, 2018 20:24
@nicolas-grekasnicolas-grekas added this to the4.2 milestoneOct 17, 2018
@ycerutoycerutoforce-pushed thetrigger_deprecation branch 3 times, most recently from1b1334f to44870efCompareOctober 19, 2018 12:31
@nicolas-grekas
Copy link
Member

offsetGet($option/, bool $triggerDeprecation = true/)

looks like this is the way to go, isn't it?

yceruto reacted with thumbs up emoji

@yceruto
Copy link
MemberAuthor

It should be ready now.

@fabpot
Copy link
Member

Thank you@yceruto.

@fabpotfabpot merged commit1af23c9 intosymfony:masterOct 24, 2018
fabpot added a commit that referenced this pull requestOct 24, 2018
…s used (yceruto)This PR was merged into the 4.2-dev branch.Discussion----------[OptionsResolver] Trigger deprecation only if the option is used| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#28848| License       | MIT| Doc PR        |symfony/symfony-docs#10496It's true that showing a deprecation message when the option is not used is a bit annoying and it would be heavy to get rid of it.Now, a deprecated option is triggered only when it's provided by the user or each time is being called from a lazy evaluation (except for deprecations based on the value, they're triggered only when provided by the user).Commits-------1af23c9 [OptionsResolver] Trigger deprecation only if the option is used
@ycerutoyceruto deleted the trigger_deprecation branchOctober 24, 2018 10:52
fabpot added a commit that referenced this pull requestOct 25, 2018
This PR was merged into the 4.2-dev branch.Discussion----------[Form] Deprecate TimezoneType regions option| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| BC breaks?    | no     <!-- seehttps://symfony.com/bc -->| Deprecations? | yes| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->| Fixed tickets |#28848| License       | MIT| Doc PR        | symfony/symfony-docs#... <!-- required for new features -->I know i've added this option myself in 4.1, but given my recent development for#28624 i realized it's an opinionated feaure, which can/should be solved on user-side (`choice_filter/choice_loader` and/or `group_by`).- blocks translations as we dont have them (see#28831)- blocks possibility of switching to Intl zones which doesnt really have this filter feature (see#28836)~While at it, i solved a few issues with `OptionsResolver` that is able to deprecate options as of 4.2 also.~ Fixed in#28878- when resolved trigger the deprecation- allow to opt-out from triggering the deprecation- dont trigger deprecation for default values (only given ones)Commits-------5cb532d [Form] Deprecate TimezoneType regions option
xabbuh added a commit to symfony/symfony-docs that referenced this pull requestNov 11, 2018
…on (yceruto)This PR was merged into the master branch.Discussion----------[OptionsResolver] Add some notes about trigger deprecationUpdates according tosymfony/symfony#28878Commits-------bb4861b Add some notes about trigger deprecation
nicolas-grekas added a commit that referenced this pull requestJun 9, 2019
…ons::offsetGet() (nicolas-grekas)This PR was submitted for the 4.3 branch but it was merged into the 4.2 branch instead (closes#31950).Discussion----------[OptionsResolver] fix adding $triggerDeprecation to Options::offsetGet()| Q             | A| ------------- | ---| Branch?       | 4.3| Bug fix?      | no| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | -| License       | MIT| Doc PR        | -This has been added by@yceruto in#28878 but it doesn't make sense to me: the interface has no concept if deprecation (`OptionsResolver` has!)Commits-------adc7e6a [OptionsResolver] fix adding $triggerDeprecation to Options::offsetGet()
javiereguiluz added a commit to symfony/symfony-docs that referenced this pull requestJun 11, 2019
…ruto)This PR was squashed before being merged into the 4.2 branch (closes#11704).Discussion----------[OptionsResolver] Add note about deprecated optionsMissing piece related tosymfony/symfony#28878Commits-------aaac426 [OptionsResolver] Add note about deprecated options
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fabpotfabpotfabpot approved these changes

@dunglasdunglasdunglas approved these changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@xabbuhxabbuhxabbuh approved these changes

+1 more reviewer

@ro0NLro0NLro0NL approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.2

Development

Successfully merging this pull request may close these issues.

7 participants

@yceruto@nicolas-grekas@fabpot@dunglas@ro0NL@xabbuh@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp