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

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
Copy link
Contributor

@OskarStark OskarStark left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍🏻

@OskarStark
Copy link
Contributor

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

@welcoMattic
Copy link
Member Author

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.

@welcoMattic welcoMattic force-pushed the fix/loco-provider branch 2 times, most recently from 93d0550 to 06a9ff1 Compare November 15, 2021 19:34
@fabpot
Copy link
Member

fabpot commented Nov 16, 2021

Thank you @welcoMattic.

@fabpot fabpot merged commit 56798bd into symfony:5.3 Nov 16, 2021
@welcoMattic welcoMattic deleted the fix/loco-provider branch November 16, 2021 08:27
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.

6 participants