Allow installing doctrine/inflector 2.0 #600
Conversation
|
For your consideration, Symfony Inflector component was deprecated and moved into the amazing String component (symfony/symfony#35092). Given that we should start using String sooner than later, we may also consider replacing Doctrine inflector by Symfony String inflector. Or we may not. I'm just commenting things, not recommending changes. Thanks! |
|
@javiereguiluz no worries. I'll leave that to someone else - I'm just offering this work to avoid breaking your CI build due to deprecations when I release 1.4 soon. If you'd rather drop doctrine/inflector in favour of symfony/string, that's for someone else to take care of. |
|
Sure! This PR probably needs to be merged without much discussion to fix the issues you mentioned. So, thanks for it! I was just commenting about what to do in the future ... in case we decide to replace our |
src/Maker/MakeCrud.php
Outdated
| $this->inflector = $inflector; | ||
|
|
||
| if (null === $inflector) { | ||
| @trigger_error(sprintf('"%s" will require a value for the $inflector argument in 2.0.', __METHOD__), E_USER_DEPRECATED); |
There was a problem hiding this comment.
This is a bit tricky. When testing with lowest deps (so doctrine/inflector 1.0), MakerBundle's config will not pass this argument, and so it triggers a deprecation. In other words: if you use MakeBundle with doctrine/inflector 1.0, you would see this deprecation no matter what (unless you upgraded doctrine/inflector to 2.0.
One easy fix would be to not add a new argument: just InflectorFactory::create()->build(); like we do with Validator. Obviously, that's not normally something we would do, but is there any practical downside?
There was a problem hiding this comment.
No practical downside. If you were to support multiple languages, you’d want to inject it, but given how we hardcode to English we may as well push this change back, at least until we require 1.4 and always have the new API.
|
Updated, let's try this version :) I agree with using String, just from a "let's eat our own dog food" standpoint (though I have zero issues with the Doctrine library). If someone makes a PR for it, that would be great. |
9b38f1b to
9ff06fc
Compare
9ff06fc to
335bd1e
Compare
|
Thank you Andreas! |
With the release of doctrine/inflector 1.4, the existing API is deprecated in favour of a new API. This PR updates the MakerBundle to provide compatibility to doctrine/inflector 1.2 (required to provide support for PHP 7.1), Inflector 1.4 (providing the new API and deprecating the legacy API), and Inflector 2.0 (which drops the legacy API).
This adds a new (currently optional) constructor argument to the crud maker. This is the only maker that gets the inflector injected instead of using it through the
Strclass (which now provides its own static maker). In the long term, this should be made consistent: the new API technically allows us to support pluralising and singularising strings in a language other than English (Inflector 1.4+ supports English, Spanish, Portuguese, and a few more). This is beyond the scope of this PR and thus omitted for the time being.NB: I saw that PHP 7.0 was dropped recently. Dropping PHP 7.1 as well would allow changing the constraint to require at least Inflector 1.4, removing the need for using the legacy inflector API and always using the new API. It is not required to make this bundle compatible with 2.0.