NotificationsYou must be signed in to change notification settings
Fork9.6k
Star30.4k

[Config] Deprecate ArrayNodeDefinition::ignoreExtraKeys in favor of s etIgnoreExtraKeys and setRemoveExtraKeys#47284

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

Jump to bottom

Open

alamirault wants to merge1 commit intosymfony:7.4

base:7.4

Choose a base branch

fromalamirault:feature/33590-deprecate-array-node-definition-ignore-extra-keys

Open

[Config] Deprecate ArrayNodeDefinition::ignoreExtraKeys in favor of s etIgnoreExtraKeys and setRemoveExtraKeys#47284

alamirault wants to merge1 commit intosymfony:7.4fromalamirault:feature/33590-deprecate-array-node-definition-ignore-extra-keys

+70 −5

Conversation

Copy link

Contributor

alamirault commentedAug 15, 2022•
edited by OskarStark
Loading

Q	A
Branch?	6.2
Bug fix?	no
New feature?	yes
Deprecations?	yes
Tickets	Fix#33590
License	MIT
Doc PR	symfony/symfony-docs#...

ArrayNodeDefinition::ignoreExtraKeys($bool) is ambiguous, it controlArrayNode::ignoreExtraKeys andArrayNode::removeExtraKeys at the same time.

Introduce unambiguous methods and deprecateignoreExtraKeys

(It's seems better than create human helper constant)

(Unrelated fabbot failure)

carsonbot added Status: Needs Review Config Deprecation Feature labels

Aug 15, 2022

carsonbot added this to the6.2 milestone

Aug 15, 2022

alamirault mentioned this pull request

Aug 15, 2022

[Config] ignoreExtraKeys(false) is misleading#33590

Closed

OskarStark changed the title~~[Config] Deprecate ArrayNodeDefinition::ignoreExtraKeys in favor of s…~~[Config] Deprecate ArrayNodeDefinition::ignoreExtraKeys in favor of s etIgnoreExtraKeys and setRemoveExtraKeys

Aug 19, 2022

alamirault force-pushed thefeature/33590-deprecate-array-node-definition-ignore-extra-keys branch from770f209 to446ced9Compare

October 29, 2022 14:07

Copy link

ContributorAuthor

alamirault commentedOct 29, 2022

@carsonbot find me a reviewer please

Copy link

carsonbot commentedOct 29, 2022

I'm sorry. I could not find any suitable reviewer.

nicolas-grekas modified the milestones:6.2,6.3

Nov 5, 2022

[Config] Deprecate ArrayNodeDefinition::ignoreExtraKeys in favor of s…

ff56741

…etIgnoreExtraKeys and setRemoveExtraKeys

alamirault force-pushed thefeature/33590-deprecate-array-node-definition-ignore-extra-keys branch from446ced9 toff56741Compare

March 10, 2023 18:44

Copy link

ContributorAuthor

alamirault commentedMar 10, 2023

I rebased branch on 6.3, I'm looking for reviewers 😄

nicolas-grekas modified the milestones:6.3,6.4

May 23, 2023

Copy link

Member

javiereguiluz commentedJun 29, 2023

I like this proposal ... but the new method names look a bit odd compared to the rest of methods:

setIgnoreExtraKeys()setRemoveExtraKeys()

Maybe we could use these names instead:

ignoreExtraKeys()removeExtraKeys()

This is a bit more complicated because we need to handle differently the deprecation of the current behavior ofignoreExtraKeys(), but it may be worth it.