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

Translate Theia using language packs #10106

Merged
merged 1 commit into from
Oct 21, 2021
Merged

Translate Theia using language packs #10106

merged 1 commit into from
Oct 21, 2021

Conversation

msujew
Copy link
Member

@msujew msujew commented Sep 15, 2021

What it does

Addresses another open issue from #9538: Localization of Theia using (mostly) vscode language packs.

Let me know if you find any strings that are user facing and not translatable yet.

Be aware that not everything is translated in here, see #10188.

How to test

  1. Download and install a vscode language pack of your choice.
  2. Run the application and look out for missing/erroneous translations.

Review checklist

Reminder for reviewers

@msujew msujew force-pushed the msujew/theia-i18n branch 2 times, most recently from eeb9516 to 4b05a4d Compare September 15, 2021 13:52
@msujew msujew added the localization issues related to localization/internalization/nls label Sep 15, 2021
@msujew msujew mentioned this pull request Sep 29, 2021
8 tasks
@msujew msujew force-pushed the msujew/theia-i18n branch 7 times, most recently from b2ddec3 to 6f05ada Compare October 18, 2021 14:28
Copy link
Contributor

@jbicker jbicker left a comment

Choose a reason for hiding this comment

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

Theia is successfully translated. Thanks!

@msujew msujew force-pushed the msujew/theia-i18n branch from 6f05ada to 6f5aa95 Compare October 21, 2021 15:34
@msujew msujew merged commit eeee962 into master Oct 21, 2021
@github-actions github-actions bot added this to the 1.19.0 milestone Oct 21, 2021
@msujew msujew deleted the msujew/theia-i18n branch October 21, 2021 18:01
@vince-fugnitto vince-fugnitto mentioned this pull request Oct 21, 2021
1 task
@vince-fugnitto
Copy link
Member

Given the impact of these changes, we might have wanted to have additional reviewers from various companies to offer input (no request for reviewers as is). I don't believe the pull-request description (or commit) documents and clarifies enough the impacts, and design.

See discussions:

@msujew
Copy link
Member Author

msujew commented Oct 22, 2021

@vince-fugnitto While it's true, that I requested no reviews from anyone via GitHub (mea culpa), I asked for reviews multiple times during the weekly dev-meetings, even confirming with @marcdumais that everything should be merged before the end of this week.

On another note, the actual design was documented in #9538 (and a bit in #9708). I.e. translating Theia using language packs where possible, which leads to the coupling of Theia and IDs used in the vscode language packs. Trying to minimize the effort of translating Theia by using the already translated vscode language packs was part of the design. This PR just addresses the open issue from #9538 that Theia wasn't actually translated using the design proposed in there.

@vince-fugnitto
Copy link
Member

@vince-fugnitto While it's true, that I requested no reviews from anyone via GitHub (mea culpa), I asked for reviews multiple times during the weekly dev-meetings, even confirming with @marcdumais that everything should be merged before the end of this week.

The problem was that I feel this should have probably been documented better in the pull-request description, commit message and in some guideline, and for the most part fell under the radar as there were many localization pull-requests opened simultaneously. I would have wanted more feedback from the community about this one (especially since we are now so tightly coupled with vscode and leak plugin information across the framework (something usually reserved for the plugin system itself).

On another note, the actual design was documented in #9538 (and a bit in #9708). I.e. translating Theia using language packs where possible, which leads to the coupling of Theia and IDs used in the vscode language packs. Trying to minimize the effort of translating Theia by using the already translated vscode language packs was part of the design. This PR just addresses the open issue from #9538 that Theia wasn't actually translated using the design proposed in there.

Unfortunately I'm not sure everyone was aware of that coupling. Given they are plugins I expected it to be isolated to the plugin-system itself and not leaked across the framework. I did not expect references to vscode keys everywhere in the codebase. That's of course my own opinion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
localization issues related to localization/internalization/nls
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants