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

Localization of GHfVS for Chinese - Simplified and Traditional #1714

Merged
merged 35 commits into from
Aug 10, 2018
Merged

Localization of GHfVS for Chinese - Simplified and Traditional #1714

merged 35 commits into from
Aug 10, 2018

Conversation

maikebing
Copy link
Contributor

@maikebing maikebing commented Jun 1, 2018 •
edited
Loading

Localization work started
Let's all translate together, click here to join https://crwd.in/github-visual-studio HTTPS://CRWD.IN/GITHUBVS

  • Chinese (Simplified) -

  • Chinese (Traditional) -

  • English -

- [ ] Czech
- [ ] French - 
- [ ] German -  @wenz
- [ ] Italian -  
- [ ] Japanese
- [ ] Korean
- [ ] Polish
- [ ] Portuguese (Brazil)
- [ ] Russian - 
- [ ] Spanish - 
- [ ] Turkish

@maikebing
Copy link
Contributor Author

maikebing commented Jun 1, 2018 •
edited
Loading

#18 Localization: Localize the extension to {insert language here}

@maikebing
Copy link
Contributor Author

1
2

@drguthals
Copy link

Thank you @maikebing for getting this started! We're really excited to see localization in our extension! 🎉

Let us know how we can help 👍

@maikebing
Copy link
Contributor Author

@sguthals GitHub context Menu I have not been able to localize, and I did not find the reason, perhaps I fell into some kind of misunderstanding. Do you know who is familiar with VSCT?

@drguthals
Copy link

Hi @maikebing!

Maybe @jcansdale or @grokys can help you out with that!

@grokys
Copy link
Contributor

grokys commented Jun 21, 2018

Hi @maikebing - to be honest this is something I've never done either!

First of all, what have you already tried? Have you tried following the instructions at https://docs.microsoft.com/en-gb/visualstudio/extensibility/localizing-menu-commands?

@maikebing
Copy link
Contributor Author

maikebing commented Jun 29, 2018 •
edited
Loading

githubchs
Hi @sguthals @grokys
The problems that have been encountered have been resolved. Please review the code? If you have any questions, please ask me.

@heku If you have time, can you reconsider the Chinese content?

@maikebing
Copy link
Contributor Author

@sguthals When and who is going to review the code?

@meaghanlewis
Copy link
Contributor

@maikebing I'll test out this PR today.

Perhaps @grokys can review the code?

@grokys
Copy link
Contributor

grokys commented Aug 10, 2018

Thanks for keeping with this @maikebing! I am reviewing now. Localization is something I'm not very familiar with, so I apologise if I need to ask stupid questions ;)

grokys
grokys previously requested changes Aug 10, 2018
View reviewed changes
Copy link
Contributor

@grokys grokys left a comment

Choose a reason for hiding this comment

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

This is looking great! I tested it, and though I cannot read Chinese and so can't attest to the translations, everything seems to be translated!

I've got just a few very small changes to request in the code. I know that this isn't the focus of this PR so I would be happy to make these changes myself - just let me know if you want me to do it.

Other than that, I think this looks good to merge!

test/GitHub.VisualStudio.UnitTests/Commands/OpenFromClipboardCommandTests.cs Outdated
@@ -50,8 +51,12 @@ public async Task UnknownLinkType()

vsServices.Received(1).ShowMessageBoxInfo(expectMessage);
}
[Test]
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: needs a space between methods.

test/GitHub.VisualStudio.UnitTests/Commands/OpenFromClipboardCommandTests.cs Outdated
{
await CouldNotResolve("TargetOwner", "CurrentOwner", Resources.NoResolveDifferentOwnerMessage);
}
[Test]
Copy link
Contributor

Choose a reason for hiding this comment

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

Also needs a newline here, and between the following methods.

test/GitHub.VisualStudio.UnitTests/Commands/OpenFromClipboardCommandTests.cs Outdated
@@ -126,9 +144,12 @@ public async Task NoChangesInWorkingDirectory()
vsServices.DidNotReceiveWithAnyArgs().ShowMessageBoxInfo(null);
gitHubContextService.Received(1).TryOpenFile(repositoryDir, context);
}

[Test]
Copy link
Contributor

Choose a reason for hiding this comment

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

And here ;)

src/GitHub.InlineReviews/Views/CommentView.xaml Outdated
@@ -67,7 +67,7 @@
CornerRadius="3"
Padding="2 1"
Visibility="{Binding IsPending, Converter={ui:BooleanToVisibilityConverter}, FallbackValue=Collapsed}">
<TextBlock FontSize="10">Pending</TextBlock>
<TextBlock FontSize="10" Text="{x:Static prop:Resources.Pending}"></TextBlock>
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that the <TextBlock> doesn't have any content, it can use an auto-closing tag, i.e.:

<TextBlock FontSize="10" Text="{x:Static prop:Resources.Pending}"/>

This can be done in a lot of places, I won't add a comment for each one ;)

@grokys
Copy link
Contributor

grokys commented Aug 10, 2018

I was worried about what would happen if we add a new resource in English but there isn't a corresponding resource in the other language yet. I just tried this and it appears to fall-back to the English text, which is the best outcome we can hope for.

@maikebing
Copy link
Contributor Author

@grokys Yes , it fall-back to English .

@meaghanlewis
Copy link
Contributor

This looks good to me. Thanks so much @maikebing! ✨

@meaghanlewis meaghanlewis dismissed grokys’s stale review August 10, 2018 22:29

changes addressed!

@meaghanlewis meaghanlewis merged commit 3b501b0 into github:master Aug 10, 2018
@meaghanlewis meaghanlewis added this to the 2.5.5 milestone Aug 10, 2018
@meaghanlewis meaghanlewis mentioned this pull request Aug 10, 2018
Closed
14 tasks
@meaghanlewis meaghanlewis changed the title Localization Localization of GHfVS for Chinese - Simplified and Traditional Aug 10, 2018
grokys added a commit that referenced this pull request Aug 14, 2018
@grokys
Fix akavache submodule.
c2c81d5 
#1714 incorrectly reverted the akavache submodule to an earlier version. Undo this.
@grokys grokys mentioned this pull request Aug 14, 2018
Merged
@meaghanlewis meaghanlewis mentioned this pull request Aug 16, 2018
Closed
@grokys grokys mentioned this pull request Aug 22, 2018
Merged
grokys added a commit that referenced this pull request Aug 22, 2018
@grokys
Revert "Merge pull request #1714 from maikebing/localization"
c8fcd9f 
This reverts commit 3b501b0, reversing
changes made to d3641f4.
@grokys grokys mentioned this pull request Aug 22, 2018
Merged
meaghanlewis pushed a commit that referenced this pull request Aug 22, 2018
Merge pull request #1872 from github/revert/1714
5e27021 
Revert pull request #1714
@meaghanlewis meaghanlewis modified the milestones: 2.5.5, 2.5.6 Aug 22, 2018
@StanleyGoldman StanleyGoldman mentioned this pull request Aug 31, 2018
Merged
@StanleyGoldman
Copy link
Contributor

Hi @maikebing!
Thank you for your help!

We tried to merge this but had some bugs.
We are trying again here in this Pull Request #1888.

I learned a lot more about CrowdIn.
I created a new CrowdIn Project and invited you: https://crwd.in/github-visual-studio
This CrowdIn Project is integrated with our GitHub Repository.

Can I ask you to resubmit your original translations in this project?

I would like to send you a private message.
I tried to send you a message on Twitter.
Should I contact you via email instead?

@maikebing
Copy link
Contributor Author

[email protected]

@grokys grokys mentioned this pull request Sep 19, 2018
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@meaghanlewis meaghanlewis meaghanlewis approved these changes

@grokys grokys grokys left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2.5.6
Development

Successfully merging this pull request may close these issues.
5 participants
@maikebing @drguthals @grokys @meaghanlewis @StanleyGoldman