Skip to content

Remove usages of transChoice() method for Symfony 5.0 #12957

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

Conversation

kane-menicou
Copy link
Contributor

@kane-menicou kane-menicou commented Jan 18, 2020

The transChoice() method and the transChoice Twig filter were removed in Symfony 5.0

Notes for reviewers: I'm not too sure about the new Pluralized Translations section, I couldn't find any other information about it in the docs so maybe it's relevant? I'm not sure if there's a better place to put it (maybe: components/translation.rst), or if it's even needed?

Pluralized Translations
-----------------------

Translation messages can be pluralized using `interval notations`_, the count
Copy link
Contributor

Choose a reason for hiding this comment

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

where the counter value to choose the right translation can be provided using

Wdyt?

Copy link
Contributor Author

@kane-menicou kane-menicou Jan 19, 2020

Choose a reason for hiding this comment

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

Thanks for the feedback, I've removed this section based on this comment: #12957 (review)

@OskarStark
Copy link
Contributor

@wouterj would you be able to review this PR? Thanks 🙏

Copy link
Member

@wouterj wouterj left a comment

Choose a reason for hiding this comment

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

Perfect! Thanks for updating the docs on this topic :)

I would remove the "Pluralized Translations" section. As far as I know, the format described there was removed in Symfony 5 (in favor of the ICU message format).

@kane-menicou
Copy link
Contributor Author

kane-menicou commented Jan 19, 2020

Thanks for the comments, based on the feedback I've removed the Pluralized Translations section, information regarding the ICU message format is specified here: https://github.com/symfony/symfony-docs/blob/5.0/translation.rst#message-format.

@kane-menicou kane-menicou requested a review from wouterj January 19, 2020 14:26
Copy link
Member

@wouterj wouterj left a comment

Choose a reason for hiding this comment

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

Thanks for the quick reaction! It's perfect now imho :)

Copy link
Member

@xabbuh xabbuh left a comment

Choose a reason for hiding this comment

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

should be merged into the 4.4 branch IMO so that we do not show usages of the deprecated API there

@xabbuh xabbuh added this to the 4.4 milestone Jan 20, 2020
@kane-menicou
Copy link
Contributor Author

@xabbuh should I change the target branch?

@xabbuh
Copy link
Member

xabbuh commented Jan 20, 2020

@kane-menicou Thank you, but we use a tool for merging that is able to do it. So it's not really necessary for you to change it. :)

@javiereguiluz javiereguiluz changed the base branch from 5.0 to 4.4 January 21, 2020 19:55
@javiereguiluz javiereguiluz force-pushed the remove-usages-of-trans-choice branch from ba613ad to 73376df Compare January 21, 2020 19:55
@javiereguiluz
Copy link
Member

Thank you Kane.

javiereguiluz added a commit that referenced this pull request Jan 21, 2020
…ane-menicou)

This PR was submitted for the 5.0 branch but it was squashed and merged into the 4.4 branch instead (closes #12957).

Discussion
----------

Remove usages of transChoice() method for Symfony 5.0

The `transChoice()` method and the `transChoice` Twig filter [were removed in Symfony 5.0](https://github.com/symfony/symfony/blob/master/UPGRADE-5.0.md#frameworkbundle)

**Notes for reviewers:** I'm not too sure about the new `Pluralized Translations` section, I couldn't find any other information about it in the docs so maybe it's relevant? I'm not sure if there's a better place to put it (maybe: `components/translation.rst`), or if it's even needed?

Commits
-------

73376df Remove usages of transChoice() method for Symfony 5.0
@javiereguiluz javiereguiluz merged commit 73376df into symfony:4.4 Jan 21, 2020
@kane-menicou kane-menicou deleted the remove-usages-of-trans-choice branch January 22, 2020 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants