Skip to content

[Translation] Fixed issue with new vs old TranslatorInterface in TranslationDataCollector #31599

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 1 commit into from
May 23, 2019

Conversation

althaus
Copy link
Contributor

@althaus althaus commented May 23, 2019

I'm not sure when this gets executed, but overriding $trans directly after the if simply looks wrong.

Q A
Branch? 4.3-beta2, but last change at that position is a couple of months
Bug fix? yes, me thinks
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT

Cheers
Matthias

@ogizanagi
Copy link
Contributor

Looks good indeed, but this should go to 4.2 where the bug was introduced (#28375)

@ogizanagi ogizanagi added this to the 4.2 milestone May 23, 2019
@nicolas-grekas nicolas-grekas changed the base branch from master to 4.2 May 23, 2019 15:21
@nicolas-grekas
Copy link
Member

Thank you @althaus.

@nicolas-grekas nicolas-grekas merged commit a1677c7 into symfony:4.2 May 23, 2019
nicolas-grekas added a commit that referenced this pull request May 23, 2019
…ace in TranslationDataCollector (althaus)

This PR was submitted for the master branch but it was squashed and merged into the 4.2 branch instead (closes #31599).

Discussion
----------

[Translation] Fixed issue with new vs old TranslatorInterface in TranslationDataCollector

I'm not sure when this gets executed, but overriding `$trans` directly after the `if` simply looks wrong.

| Q             | A
| ------------- | ---
| Branch?       | 4.3-beta2, but last change at that position is a couple of months
| Bug fix?      | yes, me thinks
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT

Cheers
Matthias

Commits
-------

a1677c7 [Translation] Fixed issue with new vs old TranslatorInterface in TranslationDataCollector
@althaus althaus deleted the patch-1 branch May 24, 2019 08:36
This was referenced May 28, 2019
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.

4 participants