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

Do not use deprecated Doctrine Inflector API #214

Merged
merged 1 commit into from
Nov 5, 2020

Conversation

pamil
Copy link
Contributor

@pamil pamil commented Nov 5, 2020

No description provided.

@pamil pamil requested a review from a team as a code owner November 5, 2020 14:03
@pamil pamil merged commit 5f452f1 into Sylius:master Nov 5, 2020
@pamil pamil deleted the component-doctrine-common branch November 5, 2020 14:06
"doctrine/doctrine-bundle": "^1.12 || ^2.0",
"doctrine/event-manager": "^1.1",
"doctrine/inflector": "^1.4 || ^2.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

This dependency will block newest resource bundle in the main Sylius repository: https://github.com/Sylius/Sylius/blob/master/composer.json#L195

We've conflicted with doctrine/inflector because of the fix of taxon word pluralization(from taxons to taxa). Proposed fixed may be found at: doctrine/inflector#147

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should fix that in Sylius itself - ^1.4 is required because that's the first version that introduces forward compatibility with ^2.0.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm just curious what's the best way to make that Inflector customisable - should we allow for setInflector static method on the metadata?

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.

None yet

2 participants