Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

I18n pull request restore with CrowdIn support #1888

Merged
merged 2 commits into from
Sep 12, 2018
Merged

Conversation

StanleyGoldman
Copy link
Contributor

@StanleyGoldman StanleyGoldman commented Aug 30, 2018

This PR restores parts of #1714 and adds initial integration with CrowdIn.

It is currently only providing translations in zn-CH for our .resx files. We're going to need a more scalable solution than duplicating the entirety of our .vsct files for every language so those files are going to be tackled in a separate PR.

The neutral language en-US resources are being stored directly in the assemblies themselves with satellite assemblies only being used for translations. This is because WPF does not work well with satellite assemblies for the neutral language.

@StanleyGoldman StanleyGoldman changed the title Internationalization I18n Aug 30, 2018
@StanleyGoldman StanleyGoldman changed the title I18n I18n pull request restore with CrowdIn support Sep 1, 2018
@grokys
Copy link
Contributor

grokys commented Sep 5, 2018

Merged #1871 into this branch, but it appears that the zh-HANS translations aren't present here. Why is that?

crowdin.yml Outdated
- source: /src/**/Resources.en-US.resx
translation: /%original_path%/Resources.%locale%.resx
- source: /src/**/VSPackage.en-US.resx
translation: /%original_path%/VSPackage.%locale%.resx
Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't do this one right now. This is information that's merged with the package for the marketplace and the extension list, not for the VS UI itself.

crowdin.yml Outdated
- source: /src/**/VSPackage.en-US.resx
translation: /%original_path%/VSPackage.%locale%.resx
- source: /src/**/OcticonPaths.en-US.resx
translation: /%original_path%/OcticonPaths.%locale%.resx
Copy link
Contributor

Choose a reason for hiding this comment

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

There is nothing to translate on this, these are icons in svg format.

@codecov
Copy link

codecov bot commented Sep 11, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@0e64838). Click here to learn what that means.
The diff coverage is 25.92%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1888   +/-   ##
=========================================
  Coverage          ?   40.88%           
=========================================
  Files             ?      487           
  Lines             ?    18912           
  Branches          ?        0           
=========================================
  Hits              ?     7733           
  Misses            ?    11179           
  Partials          ?        0
Impacted Files Coverage Δ
...Hub.InlineReviews/Views/InlineCommentPeekView.xaml 0% <ø> (ø)
...alStudio/Views/GitHubPane/PullRequestListView.xaml 0% <ø> (ø)
...isualStudio/Views/Dialog/LoginCredentialsView.xaml 0% <ø> (ø)
...udio/Views/GitHubPane/PullRequestCreationView.xaml 0% <ø> (ø)
src/GitHub.InlineReviews/Views/CommentView.xaml 0% <ø> (ø)
...Studio/Views/GitHubPane/PullRequestDetailView.xaml 0% <ø> (ø)
...GitHub.VisualStudio/Commands/OpenFromUrlCommand.cs 0% <0%> (ø)
...Studio/Views/Dialog/ForkRepositoryExecuteView.xaml 0% <0%> (ø)
....VisualStudio/Commands/OpenFromClipboardCommand.cs 95.12% <100%> (ø)
src/GitHub.VisualStudio.UI/Resources.Designer.cs 8.64% <15.9%> (ø)

@grokys grokys force-pushed the internationalization branch from dfd1f70 to 9839f9c Compare September 11, 2018 11:26
@grokys
Copy link
Contributor

grokys commented Sep 11, 2018

@maikebing could take a look at this and tell me if I've got anything wrong?

And move hard-coded UI strings to resources.

Co-Authored-By: MysticBoy <[email protected]>
@grokys grokys force-pushed the internationalization branch from 9839f9c to d45f6b1 Compare September 11, 2018 11:32
Copy link
Collaborator

@jcansdale jcansdale left a comment

Choose a reason for hiding this comment

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

Looking good! Just a suggestion about how we could handle resources in TestCase attributes and a white space issue.

@@ -126,9 +146,13 @@ public async Task NoChangesInWorkingDirectory()
vsServices.DidNotReceiveWithAnyArgs().ShowMessageBoxInfo(null);
gitHubContextService.Received(1).TryOpenFile(repositoryDir, context);
}
[Test]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing CR

Resolve resources that start with a "#" using ResolveResources.
Copy link
Collaborator

@jcansdale jcansdale left a comment

Choose a reason for hiding this comment

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

All good. 👍

Copy link
Collaborator

@jcansdale jcansdale left a comment

Choose a reason for hiding this comment

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

Approve. 😕

@maikebing
Copy link
Contributor

Looking good!

@StanleyGoldman
Copy link
Contributor Author

Where are the resources specified to be satellites?

@grokys grokys merged commit c1f15e1 into master Sep 12, 2018
@grokys
Copy link
Contributor

grokys commented Sep 12, 2018

@StanleyGoldman the non-en-US .resx resources will be put in satellite assemblies.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants