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

[Translation] [Loco] Generate id parameter instead of letting Loco do it #43990

Merged
merged 1 commit into from Nov 16, 2021

Conversation

@welcoMattic
Copy link
Member

@welcoMattic welcoMattic commented Nov 10, 2021

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

With this PR we get rid of the auto generated id from Loco (which generate id with dash notation). The counterpart is that we have to iterate over the fetched catalogues to transform the received translation keys, I'm not 100% sure about the performance impact on very large catalogues.

@carsonbot carsonbot added this to the 5.3 milestone Nov 10, 2021
@carsonbot carsonbot changed the title [Translation][Loco] Generate id parameter instead of letting Loco do it [Translation] [Loco] Generate id parameter instead of letting Loco do it Nov 10, 2021
@welcoMattic welcoMattic force-pushed the fix/loco-provider branch from b9a3310 to 18cfc68 Nov 10, 2021
Copy link
Contributor

@OskarStark OskarStark left a comment

Looks good to me 👍🏻

Loading

@OskarStark
Copy link
Contributor

@OskarStark OskarStark commented Nov 11, 2021

Is there anything "breaking" if the user now switch to the "new" logic?

Loading

@welcoMattic
Copy link
Member Author

@welcoMattic welcoMattic commented Nov 11, 2021

As it's already broken for all projects without the asset alias "name", I don't think so. And for all others projects with this asset alias, I made the check of the prefix key to check if the key starts with the domain name or not.

To be sure, I will made a test project with and without assay alias and process some tests, I'll let you know.

Loading

@welcoMattic welcoMattic force-pushed the fix/loco-provider branch 2 times, most recently from 93d0550 to 06a9ff1 Nov 15, 2021
fabpot
fabpot approved these changes Nov 16, 2021
@fabpot fabpot force-pushed the fix/loco-provider branch from 06a9ff1 to 1a44526 Nov 16, 2021
@fabpot
Copy link
Member

@fabpot fabpot commented Nov 16, 2021

Thank you @welcoMattic.

Loading

@fabpot fabpot merged commit 56798bd into symfony:5.3 Nov 16, 2021
7 of 10 checks passed
Loading
@welcoMattic welcoMattic deleted the fix/loco-provider branch Nov 16, 2021
@fabpot fabpot mentioned this pull request Nov 18, 2021
@fabpot fabpot mentioned this pull request Nov 18, 2021
@fabpot fabpot mentioned this pull request Nov 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants