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

chore(translations): fix translations order #28031

Merged
merged 5 commits into from
Apr 22, 2024

Conversation

lscheibel
Copy link
Contributor

SUMMARY

This PR does two things:

  1. fixes a use of f-strings in sql_parse.py which causes pybabel to error. (See Pybabel should raise an error when it encounters f-strings python-babel/babel#715 for why.)
  2. reorders the translation messages within the translation files by simply running the commands described in https://superset.apache.org/docs/contributing/translations/ which otherwise would have lead to large merge conflicts even for small translation changes.

This PR does not include any changes to the translations themselves.

TESTING INSTRUCTIONS

When extracting the messages from using the documented pybabel commands, without any translation changes, the message.pot file should not change (except for the license information).

ADDITIONAL INFORMATION

  • Has associated issue: Extracted Translation Order #27894
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

Fixes #27894

@github-actions github-actions bot added i18n Namespace | Anything related to localization i18n:spanish Translation related to Spanish language i18n:italian Translation related to Italian language i18n:french Translation related to French language i18n:chinese Translation related to Chinese language i18n:japanese Translation related to Japanese language i18n:russian Translation related to Russian language i18n:korean Translation related to Korean language i18n:dutch i18n:slovak i18n:ukrainian i18n:portuguese i18n:brazilian and removed size/XXL labels Apr 15, 2024
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Congrats on making your first PR and thank you for contributing to Superset! 🎉 ❤️

We hope to see you in our Slack community too! Not signed up? Use our Slack App to self-register.

@lscheibel lscheibel force-pushed the #27894-translation-order branch from b830408 to 106613c Compare April 15, 2024 11:38
@@ -765,8 +765,8 @@ def _extract_tables_from_sql(self) -> set[Table]:
SupersetError(
error_type=SupersetErrorType.QUERY_SECURITY_ACCESS_ERROR,
message=__(
f"You may have an error in your SQL statement. {message}"
),
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if @john-bodley knows a good way to lint against this. According to GPT, we can write a custom pylint checker, but it seems like that would be a first for this codebase. Something off the shelf to prevent this would be nice.

Copy link
Member

Choose a reason for hiding this comment

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

@rusackas we have custom (WIP) Pylint checkers. I guess one would need to add one to check whether there's an f-string being used within a translation __(...).

Copy link
Member

Choose a reason for hiding this comment

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

Oh, could you point me to where those live, @john-bodley, and I'll try to GPT us out of this issue?

@john-bodley john-bodley requested a review from sfirke April 16, 2024 17:21
@rusackas
Copy link
Member

Running CI 🤞

@lscheibel
Copy link
Contributor Author

Thanks for taking a look everyone. I fixed the line endings of the .po and .pot files, which were failing the CI.

@rusackas
Copy link
Member

Running Ci... then we'll merge it!

@rusackas rusackas merged commit e889f17 into apache:master Apr 22, 2024
28 checks passed
qleroy pushed a commit to qleroy/superset that referenced this pull request Apr 28, 2024
jzhao62 pushed a commit to jzhao62/superset that referenced this pull request May 16, 2024
vinothkumar66 pushed a commit to vinothkumar66/superset that referenced this pull request Nov 11, 2024
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 4.1.0 labels Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels i18n:brazilian i18n:chinese Translation related to Chinese language i18n:dutch i18n:french Translation related to French language i18n:italian Translation related to Italian language i18n:japanese Translation related to Japanese language i18n:korean Translation related to Korean language i18n:portuguese i18n:russian Translation related to Russian language i18n:slovak i18n:spanish Translation related to Spanish language i18n:ukrainian i18n Namespace | Anything related to localization size/XXL 🚢 4.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extracted Translation Order
5 participants