Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: 7.1
Choose a base branch
from

Conversation

alamirault
Copy link
Contributor

@alamirault alamirault commented Aug 15, 2022

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 control ArrayNode::ignoreExtraKeys and ArrayNode::removeExtraKeys at the same time.

Introduce unambiguous methods and deprecate ignoreExtraKeys

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

(Unrelated fabbot failure)

@carsonbot carsonbot added this to the 6.2 milestone Aug 15, 2022
@OskarStark 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 alamirault force-pushed the feature/33590-deprecate-array-node-definition-ignore-extra-keys branch from 770f209 to 446ced9 Compare October 29, 2022 14:07
@alamirault
Copy link
Contributor Author

@carsonbot find me a reviewer please

@carsonbot
Copy link

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

@nicolas-grekas nicolas-grekas modified the milestones: 6.2, 6.3 Nov 5, 2022
@alamirault alamirault force-pushed the feature/33590-deprecate-array-node-definition-ignore-extra-keys branch from 446ced9 to ff56741 Compare March 10, 2023 18:44
@alamirault
Copy link
Contributor Author

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

@nicolas-grekas nicolas-grekas modified the milestones: 6.3, 6.4 May 23, 2023
@javiereguiluz
Copy link
Member

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 of ignoreExtraKeys(), but it may be worth it.

@nicolas-grekas nicolas-grekas modified the milestones: 6.4, 7.1 Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Config] ignoreExtraKeys(false) is misleading
4 participants