Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Contracts] extract LocaleAwareInterface out of TranslatorInterface#29461

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
nicolas-grekas merged 1 commit intosymfony:4.2fromnicolas-grekas:locale-aware
Dec 6, 2018

Conversation

@nicolas-grekas
Copy link
Member

QA
Branch?4.2
Bug fix?yes
New feature?no
BC breaks?yes
Deprecations?no
Tests pass?yes
Fixed tickets-
LicenseMIT
Doc PR-

While reviewing#28929, I realized we have a design issue in the Translation contract: we missed segregatingsetLocale()/getLocale() out ofTranslatorInterface. E.g. when people type-hint forTranslatorInterface, theyshould not be auto-suggested with thesetLocale() mutator.

Technically, that's a BC break. I think we should do it. The release is close enough in time and that will save us from maintenance issues this will create in the future otherwise.

ro0NL and apfelbox reacted with thumbs up emoji
Copy link
Member

@javiereguiluzjaviereguiluz left a comment

Choose a reason for hiding this comment

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

You explained it perfectly: there may be some (little) pain in this BC change ... but it's tiny compared to its benefits.

Copy link
Member

@stofstof left a comment

Choose a reason for hiding this comment

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

The pain will only be for people having fully migrated away from the old TranslatorInterface. So I'm fine with doing that.

@nicolas-grekasnicolas-grekas merged commit73e4a1a intosymfony:4.2Dec 6, 2018
nicolas-grekas added a commit that referenced this pull requestDec 6, 2018
…Interface (nicolas-grekas)This PR was merged into the 4.2 branch.Discussion----------[Contracts] extract LocaleAwareInterface out of TranslatorInterface| Q             | A| ------------- | ---| Branch?       | 4.2| Bug fix?      | yes| New feature?  | no| BC breaks?    | yes| Deprecations? | no| Tests pass?   | yes| Fixed tickets | -| License       | MIT| Doc PR        | -While reviewing#28929, I realized we have a design issue in the Translation contract: we missed segregating `setLocale()`/`getLocale()` out of `TranslatorInterface`. E.g. when people type-hint for `TranslatorInterface`, they *should not* be auto-suggested with the `setLocale()` mutator.Technically, that's a BC break. I think we should do it. The release is close enough in time and that will save us from maintenance issues this will create in the future otherwise.Commits-------73e4a1a [Contracts] extract LocaleAwareInterface out of TranslatorInterface
@fabpotfabpot mentioned this pull requestDec 6, 2018
@Isengo1989
Copy link

Isengo1989 commentedDec 6, 2018
edited
Loading

Shouldn`t be the new Service be autowired by default? I updated my project and switched the setLocale() of the TranslatorInterface with the LocaleAwareInterface but I am getting:

Cannot autowire argument $locale of "App\Controller\UsersController::test()": it references interface "Symfony\Contracts\Translation\LocaleAwareInterface" but no such service exists.

when using$locale->setLocale('en');

slaur reacted with heart emoji

@JensGck
Copy link

Shouldn`t be the new Service be autowired by default? I updated my project and switched the setLocale() of the TranslatorInterface with the LocaleAwareInterface but I am getting:

Cannot autowire argument $locale of "App\Controller\UsersController::test()": it references interface "Symfony\Contracts\Translation\LocaleAwareInterface" but no such service exists.

when using$locale->setLocale('en');

Just define the interface in services yaml. I'm not sure if this is the best way, but it will work:

Symfony\Contracts\Translation\LocaleAwareInterface:    alias: 'translator.default'
Isengo1989 and slaur reacted with thumbs up emoji

@gmponos
Copy link
Contributor

gmponos commentedDec 24, 2018
edited
Loading

@nicolas-grekas I recently created a new service that I wanted to make itaware of the locale.

Let's say my service wasEmailSender. I tried to have this class implementLocaleAwareInterface but since this interface is underTranslation namespace it does not feel correct to have a service aboutemails implement anInterface for translation. Furthermore the locale is not used there to define the translation but to define the receiver.

So I was wondering if this could be improved even further and have theLocaleAwareInterface somewhere else.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@javiereguiluzjaviereguiluzjaviereguiluz approved these changes

@stofstofstof approved these changes

+2 more reviewers

@TobionTobionTobion approved these changes

@ogizanagiogizanagiogizanagi approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.2

Development

Successfully merging this pull request may close these issues.

9 participants

@nicolas-grekas@Isengo1989@JensGck@gmponos@javiereguiluz@stof@Tobion@ogizanagi@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp