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

IBX-9444: Fixed locale ach-UG #91

Open
wants to merge 3 commits into
base: 4.6
Choose a base branch
from

Conversation

reithor
Copy link

@reithor reithor commented Jan 29, 2025

🎫 Issue IBX-9444

From what I see we are not using the language ach anywhere - only the locale ach-UG .

PR excludes ach-UG from choices list - and also changes AvailableLocaleChoiceLoaderTest.php accordingly.

@reithor reithor requested review from mikadamczyk and a team January 29, 2025 13:25
@reithor reithor changed the base branch from main to 4.6 January 29, 2025 13:29
@reithor reithor requested a review from ViniTou January 31, 2025 08:27
@@ -19,7 +19,7 @@ class AvailableLocaleChoiceLoader implements ChoiceLoaderInterface
{
// Acholi dialect is used by In-Context translation
// and should not be present on the list of available translations.
private const EXCLUDED_TRANSLATIONS = ['ach'];
private const EXCLUDED_TRANSLATIONS = ['ach-UG'];
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure how Crowdin does things here, but just to be sure, I would leave both values if that solves the issue.
@mikadamczyk, could you sheed some light on this as well.

Copy link
Author

Choose a reason for hiding this comment

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

Hi @ViniTou ,
from what I see:

There are several alternative options:

  • Add mapping ach-UG -> ach to TranslationCollectorPass. Then there is no need at all to change anything in ibexa/user bundle
  • Or ('better safe then sorry' approach) : add both strings to EXCLUDED_TRANSLATIONS :
  private const EXCLUDED_TRANSLATIONS = ['ach', 'ach-UG'];

Copy link
Contributor

Choose a reason for hiding this comment

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

@reithor you are right, currently we have ach-UG as a locale and it should be

private const EXCLUDED_TRANSLATIONS = ['ach-UG']

The 'ach' version is only valid when someone adds it as a user_preferences.additional_translations which I bet will never happen

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.

5 participants