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

Separate pt-BR and pt-PT through a crowdin custom mapping #14947

Merged
merged 6 commits into from
Mar 8, 2024

Conversation

oliverguenther
Copy link
Member

@oliverguenther oliverguenther commented Mar 7, 2024

Previously, both pt-BR and pt-PT in crowdin were active but mapped to the same pt.yml file. pt-BR "won" that and resulted in the only portuguese language being available.

On Crowdin, I added a custom mapping that would export both in the above locales like we already did for zh-TW and zh-CN. It requires a tiny migration to replace previous occurrrences of pt in user languages and default locales.

While I was adding to the default set of languages, I went through the list in crowdin and added all that are somewhat translated. Although I do believe we should add all languages (possibly with a warning if they are not translated well so far).

https://community.openproject.org/work_packages/53374/activity

To be able to reason about the default language selection, I've added that as a "Feature" so it turns up in the release notes, even though it's just that one line change.
https://community.openproject.org/work_packages/53378

Copy link

github-actions bot commented Mar 7, 2024

1 Warning
⚠️ This PR has migration-related changes on a release branch. Ping @opf/operations

Generated by 🚫 Danger

@oliverguenther oliverguenther force-pushed the fix/portuguese-locales branch from 30f3049 to 6023119 Compare March 7, 2024 19:20
Add all translations that have active translators, and are at or around 50% translation ratio.

Do we perhaps want to enable all to improve translations, and mark those that have a bad ratio?
@oliverguenther oliverguenther force-pushed the fix/portuguese-locales branch from 6023119 to acde5de Compare March 8, 2024 08:00
@cbliard
Copy link
Member

cbliard commented Mar 8, 2024

@ulferts please do not merge yet, I'm reviewing too.

Copy link
Member

@cbliard cbliard left a comment

Choose a reason for hiding this comment

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

With the proposed changes, the language name for "pt-BR" would be "Português".
It should be "Português do Brasil" ot "Português (Brasil)".

I'll have a look at the generation script to see if it can resolve this properly.

#
# The translations come from version 42 of the Unicode CLDR project .
#
# The Unicode Common Locale Data Repository (CLDR) provides key building
# blocks for software to support the world's languages, with the largest
# and most extensive standard repository of locale data available.
---
pt:
pt-BR:
cldr:
language_name: Português
Copy link
Member

Choose a reason for hiding this comment

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

It feels wrong. It should mention Brazil somewhere.

Actually, in the CLDR, there is no language information for pt-BR from file common/main/pt_BR.xml, and the way the script works fall back to pt locale from common/main/pt.xml file instead. That's why it's "Português".

The translations for variations "pt-BR" and "pt-PT" do not exist in common/main/pt.xml, but do exist in common/main/pt_PT.xml:

  <language type="pt_BR">português do Brasil</language>
  <language type="pt_PT">português europeu</language>

The correct language name should be "Português do Brasil".

We could also use the "territory" information from common/main/pt.xml to generate the names:

  <territory type="BR">Brasil</territory>
  ...
  <territory type="PT">Portugal</territory>

Names could then be "Português (Brasil)" and "Português (Portugal)".

Which one would be better?

The script should be modified to be better at automatically resolving those edge cases.

Copy link
Member

@cbliard cbliard Mar 8, 2024

Choose a reason for hiding this comment

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

I modified the script to use "Português do Brasil" instead and commited it.

@cbliard
Copy link
Member

cbliard commented Mar 8, 2024

@ulferts you can review a second time now, and merge if ok.

@oliverguenther
Copy link
Member Author

Thanks @cbliard, I verified your changes

@oliverguenther oliverguenther merged commit 2140555 into release/13.4 Mar 8, 2024
10 of 11 checks passed
@oliverguenther oliverguenther deleted the fix/portuguese-locales branch March 8, 2024 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants