-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
Add zh-CN i18n support for Windows installer #2569
Conversation
<String Id="WelcomeDlgDescription">本安装向导将在您的计算机上安装 [ProductName]。</String> | ||
<String Id="InstallDirDlgDescription">请选择一个自定义的安装位置,或单击下一步开始安装。</String> | ||
|
||
<String Id="MajorUpgrade_DowngradeErrorMessage">已安装的 [ProductName] 的一个更高版本。安装程序将立即退出。</String> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
已安装的xxx的 去掉前一个的
Thank you @JacksonTian for reviewing. Typo fixed. |
<String Id="DocumentationShortcuts_Description">在开始菜单内添加 [ProductName] [FullVersion] 的在线文档链接以及 [ProductName] 的网站链接。</String> | ||
|
||
<String Id="EnvironmentPath_Title">添加到 PATH 环境变量</String> | ||
<String Id="EnvironmentPath_Description">把 [ProductName], npm 以及以全局方式安装的 npm 模块添加到 PATH 环境变量。</String> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
以及以
cc @nodejs/iojs-cn |
Seems good. |
LGTM |
@joaocgreis I think you are the most familiar with the windows installer bits, could you review that? |
LGTM cc @fhemberger |
<String Id="EnvironmentPathNode_Title">Node.js 和 npm</String> | ||
<String Id="EnvironmentPathNode_Description">把 [ProductName] 和 npm(如果安装了)添加到 PATH 环境变量中。</String> | ||
|
||
<String Id="EnvironmentPathNpmModules_Title">npm 模块</String> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the correct/better translation about the often word "module"? "模块" vs. "模组", as for me, +1 on "模组" :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just Taiwan or Hongkong use "模组" for module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I see...LGTM now
👍 |
Wow, awesome! I was planning to write an issue for all translators about the Windows installer. You were faster. ;) LGTM |
@nodejs/collaborators Can someone please merge this? It has no direct influence on the build (translations are still commented out). |
I might run a test build of it to get feedback from native speakers, hold on |
ah, it's kind of old, @pmq20 would you mind rebasing this off master please? |
@rvagg Sure. Rebased. |
@rvagg can I land this? If you still want to get some feedback, note that this is being committed commented in |
go ahead @joaocgreis, I put out a call but am happy to trust @pmq20 on this, I'm sure we'll get feedback on anything that can be tweaked. |
This builds well with the translation commented, but fails otherwise with the same problem as #4647 (comment) . Upstream WiX has a translation for German, but none for Chinese or Italian. A possible workaround seems to be adding those variables to the translation file, translating the original. The best would be to submit it upstream. @fhemberger do you know of a better solution for this? |
@joaocgreis Sounds good to me. Sorry, I'm no expert on this topic either. I dug into WiX once over a weekend for the initial German translation but never used it before. |
@mcollina yes we can, so we can finally have the italian version :-) |
7da4fd4
to
c7066fb
Compare
@nodejs/build is there something keeping this from landing? |
@thealphanerd This can be merged, as the translation is still commented, so nothing will break. So we still need a PR for the WiX toolset to add Chinese, Italian, et. al. |
LGTM. |
Can't believe that this is still not merged. |
@piccoloaiutante did the PR to WiX for Italian (wixtoolset/wix3#366). Did an equivalent PR happen for Chinese as well? |
@joaocgreis @mcollina Sure. Thanks for the Italian example. Let me do the same thing for the Chinese language. This PR is lasting too long to be merged. Let me first merge the translation (which has been thoroughly reviewed) and then create new PR's for the translations to take effect. |
PR-URL: nodejs#2569 Reviewed-By: Jackson Tian <[email protected]> Reviewed-By: Wexpo Lyu <[email protected]> Reviewed-By: Yiyu He <[email protected]> Reviewed-By: Yorkie Liu <[email protected]>
Landed in e09c62a |
PR-URL: #2569 Reviewed-By: Jackson Tian <[email protected]> Reviewed-By: Wexpo Lyu <[email protected]> Reviewed-By: Yiyu He <[email protected]> Reviewed-By: Yorkie Liu <[email protected]>
PR-URL: #2569 Reviewed-By: Jackson Tian <[email protected]> Reviewed-By: Wexpo Lyu <[email protected]> Reviewed-By: Yiyu He <[email protected]> Reviewed-By: Yorkie Liu <[email protected]>
PR-URL: #2569 Reviewed-By: Jackson Tian <[email protected]> Reviewed-By: Wexpo Lyu <[email protected]> Reviewed-By: Yiyu He <[email protected]> Reviewed-By: Yorkie Liu <[email protected]>
@pmq20 I've added this to v4.x-staging. Would you be able to test and make sure it works as expected before we do another release |
PR-URL: #2569 Reviewed-By: Jackson Tian <[email protected]> Reviewed-By: Wexpo Lyu <[email protected]> Reviewed-By: Yiyu He <[email protected]> Reviewed-By: Yorkie Liu <[email protected]>
PR-URL: #2569 Reviewed-By: Jackson Tian <[email protected]> Reviewed-By: Wexpo Lyu <[email protected]> Reviewed-By: Yiyu He <[email protected]> Reviewed-By: Yorkie Liu <[email protected]>
This is a follow-up on #2247 and #819
@JacksonTian Could you review the translations? thanks!