Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

chore: Localization of Superset pt. 1 #22150

Merged
merged 20 commits into from
Jan 11, 2023
Merged

chore: Localization of Superset pt. 1 #22150

merged 20 commits into from
Jan 11, 2023

Conversation

artemonsh
Copy link
Contributor

@artemonsh artemonsh commented Nov 17, 2022

SUMMARY

The localization of some components and especially charts is not complete yet. This PR is the effort to achieve the full localization of Superset. Additionally, it is a real pain to upgrade Superset as one should solve tons of merge conflicts, if they wrapped everything in t() and __() manually, while it is not reflected in the official repo :/

ADDITIONAL INFORMATION

  • Has associated issue:
  • 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

@artemonsh artemonsh changed the title Localization of some charts and elements chore: Localization of some charts and elements Nov 17, 2022
@artemonsh
Copy link
Contributor Author

@villebro take a look please

@artemonsh artemonsh changed the title chore: Localization of some charts and elements chore: Localization of several charts and elements Nov 29, 2022
@artemonsh
Copy link
Contributor Author

@michael-s-molina could you take a look please?

@codecov
Copy link

codecov bot commented Dec 15, 2022

Codecov Report

Merging #22150 (4b7d0a6) into master (73e53fa) will decrease coverage by 11.25%.
The diff coverage is 77.77%.

❗ Current head 4b7d0a6 differs from pull request most recent head 436723e. Consider uploading reports for the commit 436723e to get more accurate results

@@             Coverage Diff             @@
##           master   #22150       +/-   ##
===========================================
- Coverage   67.10%   55.84%   -11.26%     
===========================================
  Files        1869     1869               
  Lines       71580    71582        +2     
  Branches     7806     7806               
===========================================
- Hits        48031    39978     -8053     
- Misses      21521    29576     +8055     
  Partials     2028     2028               
Flag Coverage Δ
hive ?
mysql ?
postgres ?
presto 52.50% <100.00%> (+<0.01%) ⬆️
python 57.88% <100.00%> (-23.44%) ⬇️
sqlite ?
unit 51.48% <83.33%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...-chart-controls/src/sections/advancedAnalytics.tsx 14.28% <ø> (ø)
...rt-controls/src/shared-controls/sharedControls.tsx 55.17% <ø> (ø)
...ui-core/src/chart/components/FallbackComponent.tsx 100.00% <ø> (ø)
...et-ui-core/src/chart/components/SuperChartCore.tsx 100.00% <ø> (ø)
...s/legacy-plugin-chart-calendar/src/controlPanel.ts 50.00% <ø> (ø)
...s/legacy-plugin-chart-heatmap/src/controlPanel.tsx 57.14% <ø> (ø)
...ns/legacy-plugin-chart-horizon/src/controlPanel.ts 100.00% <ø> (ø)
...ns/legacy-plugin-chart-map-box/src/controlPanel.ts 30.00% <0.00%> (ø)
...legacy-plugin-chart-partition/src/controlPanel.tsx 25.00% <ø> (ø)
...egacy-plugin-chart-pivot-table/src/controlPanel.ts 100.00% <ø> (ø)
... and 386 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Deleting a tab will remove all content within it. You may still
reverse this action with the <b>undo</b> button (cmd + z) until you
save your changes.
{t(
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 template literals would make this block a little more legible/parsable. Not a dealbreaker.

@rusackas
Copy link
Member

This generally looks good to me! I added a couple clarifying questions. In general, there seems like only one real pint of contention, which is that I'm not sure we want to include the colon-space combo in the translated strings, as the word(s) without the punctuation may provide utility in other settings. For example, name: is translated, but we might just want to leverage name which is probably already translated. I think this applies to several instances. Curious if others with experience in translations (e.g. @geido @ktmud @etr2460) might feel differently.

@michael-s-molina
Copy link
Member

@rusackas According to this article it seems sometimes the punctuation could also be translated. Check the Punctuation section.

@rusackas
Copy link
Member

@rusackas According to this article it seems sometimes the punctuation could also be translated. Check the Punctuation section.

Makes sense... good enough for me. I'll give this my approval, but would love input on the other open questions, and more opinions on the matter before merging.

@artemonsh artemonsh requested a review from rusackas December 20, 2022 14:09
Copy link
Member

@rusackas rusackas left a comment

Choose a reason for hiding this comment

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

Approving updates... trying to get CI to pass.

@artemonsh artemonsh requested a review from rusackas December 23, 2022 07:12
@rusackas
Copy link
Member

rusackas commented Jan 6, 2023

Approved CI to run. Fingers crossed!

Latest changes seem fine too.

I'll circle back to merge this if CI passes. If it doesn't, nag me here or on Slack, and we'll sort it out!

@artemonsh artemonsh requested a review from rusackas January 9, 2023 08:19
@rusackas rusackas closed this Jan 9, 2023
@rusackas rusackas reopened this Jan 9, 2023
@artemonsh artemonsh closed this Jan 11, 2023
@artemonsh artemonsh reopened this Jan 11, 2023
@artemonsh artemonsh closed this Jan 11, 2023
@artemonsh artemonsh reopened this Jan 11, 2023
@rusackas rusackas merged commit f3696ce into apache:master Jan 11, 2023
@artemonsh artemonsh changed the title chore: Localization of several charts and elements chore: Localization of Superset pt. 1 Feb 7, 2023
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.1.0 and removed 🚢 2.1.3 labels Mar 13, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/XXL 🚢 2.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants