Skip to content

[Loco] Fix Loco Provider ID and pull & push local messages reading #43967

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
Nov 8, 2021

Conversation

welcoMattic
Copy link
Member

@welcoMattic welcoMattic commented Nov 8, 2021

Q A
Branch? 5.3
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #42395 & #43954
License MIT
Doc PR

It fixes Loco push new messages and avoiding Loco auto-generation of ID for each new messages (which use dash notation instead of dot notation IIRC).

And it fixes also the Translation push & pull commands when they read local messages for multiple domains.

@carsonbot carsonbot added this to the 5.4 milestone Nov 8, 2021
@welcoMattic welcoMattic changed the title Fix Loco Provider [Translation][Loco] Fix Loco Provider ID and pull & push local messages reading Nov 8, 2021
@carsonbot carsonbot changed the title [Translation][Loco] Fix Loco Provider ID and pull & push local messages reading [Loco] Fix Loco Provider ID and pull & push local messages reading Nov 8, 2021
@fabpot
Copy link
Member

fabpot commented Nov 8, 2021

Thank you @welcoMattic.

@fabpot fabpot merged commit 8a3cc18 into symfony:5.4 Nov 8, 2021
@welcoMattic welcoMattic deleted the fix/loco-provider branch November 8, 2021 18:33
@welcoMattic
Copy link
Member Author

welcoMattic commented Nov 8, 2021

@fabpot I just realized that I opened this PR on 5.4 instead of 5.3, since it is a bugfix. Sorry

@@ -171,6 +171,7 @@ private function createAssets(array $keys): array
foreach ($keys as $key) {
$responses[$key] = $this->client->request('POST', 'assets', [
'body' => [
'id' => $key,
Copy link
Member

Choose a reason for hiding this comment

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

be careful. Translation keys are only unique per domain, not globally. so they might note be suitable as loco ids.

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right, so may be we can concatenate the domain as prefix or suffix to the key?

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.

[Translation] With Loco provider, push command replaces dots with hyphens
4 participants