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

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 force-pushed the internationalization branch from 7adfbf5 to 0661a30 Aug 31, 2018
@StanleyGoldman StanleyGoldman changed the title I18n I18n pull request restore with CrowdIn support Sep 1, 2018
@grokys
Copy link
Contributor

@grokys 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

This comment has been minimized.

@shana

shana Sep 6, 2018
Collaborator

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

This comment has been minimized.

@shana

shana Sep 6, 2018
Collaborator

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

scripts/modules.ps1 Outdated Show resolved Hide resolved
@shana
shana approved these changes Sep 6, 2018
@codecov
Copy link

@codecov 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 Sep 11, 2018
@grokys
Copy link
Contributor

@grokys 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 <mysticboy@live.com>
@grokys grokys force-pushed the internationalization branch from 9839f9c to d45f6b1 Sep 11, 2018
Copy link
Collaborator

@jcansdale jcansdale left a comment

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]

This comment has been minimized.

@jcansdale

jcansdale Sep 11, 2018
Collaborator

Missing CR

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

@jcansdale jcansdale left a comment

All good. 👍

Copy link
Collaborator

@jcansdale jcansdale left a comment

Approve. 😕

@maikebing
Copy link
Contributor

@maikebing maikebing commented Sep 11, 2018

Looking good!

@StanleyGoldman
Copy link
Contributor Author

@StanleyGoldman StanleyGoldman commented Sep 11, 2018

Where are the resources specified to be satellites?

@grokys grokys merged commit c1f15e1 into master Sep 12, 2018
2 checks passed
2 checks passed
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@grokys
Copy link
Contributor

@grokys grokys commented Sep 12, 2018

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.