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

Generate dist files for new translations #528

Merged
merged 1 commit into from
Jan 17, 2025

Conversation

ricardotejedorsanz
Copy link
Contributor

Opening a separate PR to generate dist files that were causing argo test to fail in #524 + quoting "ON" in polish to avoid script parsing error.

@ricardotejedorsanz ricardotejedorsanz self-assigned this Jan 16, 2025
@ricardotejedorsanz ricardotejedorsanz added taxonomy-tree-requests Suggestion to improve to the taxonomy tree 2025-01 labels Jan 16, 2025
Copy link
Collaborator

@danielpgross danielpgross left a comment

Choose a reason for hiding this comment

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

Looks like the behaviour for sorting Other isn't working correctly -- I looked at fr ("Autre") and see that it moved from the end of the file up next to others starting with "A".

I'll look into fixing this now and will post here once the fix is merged, so we can generate it again.

@danielpgross
Copy link
Collaborator

Fix is in #529

danielpgross added a commit that referenced this pull request Jan 16, 2025
In #528, we discovered that new localized `dist` output for `Value`s had a different sort order than existing files.

The reason turned out to be that the old implementation sorted localized values according to their English names, while the new one sorted according to the localized names.

This PR changes the sort logic to match the previous behaviour: sorting by the English name, regardless of the localization. It also adds tests to cover this case.

I tested generating `dist` files for values with the `fr` locale and the changes were as expected.
@ricardotejedorsanz ricardotejedorsanz force-pushed the new-locales-updated-dist-files branch from 6cdb1a1 to 0b834d9 Compare January 17, 2025 09:54
@ricardotejedorsanz
Copy link
Contributor Author

Thanks @danielpgross --pushing another commit after the fix (LGTM).

Copy link
Collaborator

@danielpgross danielpgross left a comment

Choose a reason for hiding this comment

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

Looks great now 🚀

Let's be sure to squash the commits into a single one either before or as part of the merge into main, since the first commit has a huge diff that we don't need to keep.

@ricardotejedorsanz ricardotejedorsanz force-pushed the new-locales-updated-dist-files branch from 8ee02ca to a17af17 Compare January 17, 2025 15:27
@ricardotejedorsanz ricardotejedorsanz merged commit 6ad505b into main Jan 17, 2025
5 checks passed
@ricardotejedorsanz ricardotejedorsanz deleted the new-locales-updated-dist-files branch January 17, 2025 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2025-01 taxonomy-tree-requests Suggestion to improve to the taxonomy tree
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants