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

Fix Symfony setDeprecated() arguments #762

Closed
wants to merge 1 commit into from
Closed

Fix Symfony setDeprecated() arguments #762

wants to merge 1 commit into from

Conversation

koekaverna
Copy link

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? yes
Tests pass? yes
Documented? no
Fixed tickets #...
License MIT

Since symfony/config 5.1: The signature of method "Symfony\Component\Config\Definition\Builder\NodeDefinition::setDeprecated()" requires 3 arguments: "string $package, string $version, string $message", not defining them is deprecated.

@murtukov
Copy link
Member

murtukov commented Oct 8, 2020

@koekaverna I already fixed this in #715 and it was merged into 0.13. It just needs to be applied to master too. I just don't want to have different fixes in different versions. I'm gonna close this PR and open another one with the same patch as in the #715.

Or, you could change your PR to match the code in #715 and I approve it.

@koekaverna
Copy link
Author

@murtukov Okay, do it in your way.

@koekaverna koekaverna closed this Oct 9, 2020
@koekaverna
Copy link
Author

@murtukov
when are you planning to do it?

@murtukov
Copy link
Member

@koekaverna the deprecation message says:

The "%path%.%node%" configuration is deprecated since version 0.13 and will be removed in 1.0

The next major version is 1.0 (current master), so we don't need this BC-layer anymore, but need to remove the 'resolver_maps' option instead (as stated in the message). I guess this was the reason I didn't merge it into master.

@mcg-web can we remove this deprecated option in the master?

->arrayNode('resolver_maps')
->defaultValue([])
->prototype('scalar')->end()
->setDeprecated('The "%path%.%node%" configuration is deprecated since version 0.13 and will be removed in 1.0. Add the "overblog_graphql.resolver_map" tag to the services instead.')
->end()

@murtukov murtukov reopened this Oct 12, 2020
@murtukov murtukov closed this Oct 12, 2020
@mcg-web
Copy link
Member

mcg-web commented Oct 12, 2020

@murtukov since next major version is the 1.0 we can safely make this obsolete and remove the configuration.

@murtukov
Copy link
Member

@koekaverna see #763

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants