Skip to content

Handle deprecations from Doctrine Inflector #3564

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

Merged
merged 2 commits into from
May 28, 2020
Merged

Handle deprecations from Doctrine Inflector #3564

merged 2 commits into from
May 28, 2020

Conversation

derrabus
Copy link
Contributor

@derrabus derrabus commented May 16, 2020

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tickets #2626
License MIT

This is an alternative approach to #3560. Since Doctrine Inflector cannot be bumped to version 1.4 without dropping support for php 7.1, this PR introduces a façade that uses the new API if it's available and falls back to the 1.3 API otherwise.

This change removed a lot of messages from my application's deprecation log.

@alanpoulain
Copy link
Member

Don't you think it would be nice to use an interface to change easily the implementation (for instance if we want to use Symfony String)?

@derrabus
Copy link
Contributor Author

Don't you think it would be nice to use an interface to change easily the implementation (for instance if we want to use Symfony String)?

First of all, you could very well hide Symfony String behind the façade I'm sugesting here as well.

Yes, I think an interface would be nice. But in order to use such an interface effectively, the inflector shouldn't be called statically but injected via DI instead. That would be a larger change. I'd suggest wait with that change until php 7.1 support is dropped so we actually have access to Symfony String or the non-static Doctrine Inflector.

My goal was to provide a minimal solution, so a bugfix release could be shipped that does not trigger deprecation warnings anymore.

Copy link
Member

@soyuka soyuka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this solution for now, would you be able to rebase fix my comments and rebase?

@soyuka
Copy link
Member

soyuka commented May 26, 2020

Don't you think it would be nice to use an interface to change easily the implementation (for instance if we want to use Symfony String)?

Definitely agree on this but this would be considered as a new feature as we'd need to change more code and add some dependency injection. We need to turn our tests to green and to keep on supporting php 7.1 for now.

@derrabus
Copy link
Contributor Author

I like this solution for now, would you be able to rebase fix my comments and rebase?

Done.

@dunglas dunglas merged commit 43bddc1 into api-platform:2.5 May 28, 2020
@dunglas
Copy link
Member

dunglas commented May 28, 2020

Thank you very much @derrabus!

@derrabus derrabus deleted the bugfix/doctrine-inflector-deprecation branch May 28, 2020 18:19
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.

4 participants