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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix pt-BR issue #8523

Merged
merged 1 commit into from Nov 18, 2021
Merged

Fix pt-BR issue #8523

merged 1 commit into from Nov 18, 2021

Conversation

froger
Copy link
Contributor

@froger froger commented Nov 17, 2021

馃帺 What? Why?

Helping on #6221 (comment) pt-BR issue.
pt-BR yml files have the pt: prefix, and not the expected pt-BR one.

My guess is that with the current crowdin.yml config:

        pt-BR: pt-BR
        pt-PT: pt

Crowdin tries to have a 2 letters fallback, and as it is not provided, will take the first variant (here pt-BR), and thus is ending having pt: translated with pt-BR

So as in french, I propose to change it to have pt: pt to make sure default pt-BR is considered as a variant.

馃搶 Related Issues

Link your PR to an issue

Testing

Describe the best way to test or validate your PR.

@froger
Fix pt-BR issue
0aac9d7 
Helping on decidim#6221 (comment) pt-BR issue. 
My guess is that with the current config: 
```
        pt-BR: pt-BR
        pt-PT: pt
```
Crowdin tries to have a 2 letters fallback, and as it is not provided, will take the first option (here pt-BR), ending having `pt-BR` labeled with `pt:`

So as in french, I propose to change it to have `pt: pt` to make sure default `pt-BR` is considered as a variant.
microstudi
microstudi approved these changes Nov 17, 2021
View reviewed changes
Copy link
Contributor

@microstudi microstudi left a comment

Choose a reason for hiding this comment

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

馃憤

andreslucena
andreslucena approved these changes Nov 18, 2021
View reviewed changes
Copy link
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

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

Let's try it! 馃憤馃徑

@andreslucena andreslucena added the type: fix PRs that implement a fix for a bug label Nov 18, 2021
@andreslucena andreslucena merged commit 03800aa into decidim:develop Nov 18, 2021
@andreslucena
Copy link
Member

Seems like this didn't work @froger @microstudi - I made a small change in pt-BR to force a sync with this locale (#8527) but it didn't change the locale. Any other ideas? I can give you any crowdin panel screenshot that you need to debug this one (@microstudi has access too)

Relevant docs: https://support.crowdin.com/configuration-file/#language-mapping

@froger
Copy link
Contributor Author

froger commented Nov 19, 2021

@andreslucena thanks for trying :D
I don't know where the configuration is, I don't find it on the repo. Could you send me this file in MP ?

I'll open a crowdin-cli issue today, they might help.

@froger froger deleted the patch-1 branch November 22, 2021 10:23
@froger froger mentioned this pull request Nov 22, 2021
Closed
@microstudi
Copy link
Contributor

The problem is that I18n requires the yaml files to have the first key as the language. In this PR I don't see such change:
image

in this case the pt-BR.yml should start with pt-BR:

@froger
Copy link
Contributor Author

froger commented Nov 24, 2021 •
edited

@microstudi thanks for the clarifications.
yes, I didn't change this because the crowdin bot had change it 17months ago (see the bot PR: 745a7da)
Not sure it will change something, but we can try to change manually the yml (there are around 24files).

Screen Shot 2021-11-24 at 11 50 31

@microstudi
Copy link
Contributor

That pr looks indeed wrong. I'm pretty sure that a manual override will solve the issue.
In any case, this should be reported as a bug issue. Maybe some configuration in crowdin caused that automated pr

@andreslucena andreslucena mentioned this pull request Nov 25, 2021
Closed
@andreslucena
Copy link
Member

In any case, this should be reported as a bug issue

馃憤馃徑 I created #8549

andreslucena added a commit that referenced this pull request Nov 25, 2021
@andreslucena
Revert "Fix pt-BR issue (#8523)"
dc3e80e 
This reverts commit 03800aa.
@andreslucena andreslucena mentioned this pull request Nov 25, 2021
Closed
@alecslupu alecslupu added this to the 0.26.0 milestone Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andreslucena andreslucena andreslucena approved these changes

@microstudi microstudi microstudi approved these changes

Assignees
No one assigned
Labels
module: core target: developer-experience type: fix PRs that implement a fix for a bug
Projects
None yet
Milestone
0.26.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@froger @andreslucena @microstudi @alecslupu