Skip to content

Navigation Menu

Sign in

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
Appearance settings

[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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: 7.3
Choose a base branch
Loading
from
Open

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

wants to merge 1 commit into from

Conversation

alamirault
Copy link
Contributor

@alamirault alamirault commented Aug 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 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
@alamirault alamirault mentioned this pull request Aug 15, 2022
Closed
@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
@xabbuh xabbuh modified the milestones: 7.1, 7.2 May 15, 2024
@fabpot fabpot modified the milestones: 7.2, 7.3 Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Config Deprecation Feature Status: Needs Review
Projects
None yet
Milestone
7.3
Development

Successfully merging this pull request may close these issues.

[Config] ignoreExtraKeys(false) is misleading
6 participants
@alamirault @carsonbot @javiereguiluz @fabpot @nicolas-grekas @xabbuh
Morty Proxy This is a proxified and sanitized view of the page, visit original site.