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

Rename zh.json to zh_CN.json #511

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@andyhu
Contributor

andyhu commented May 20, 2016

This is for simplified Chinese so the correct locale should be zh_CN. For traditional Chinese it should be something like zh_HK or zh_TW.

Rename zh.json to zh_CN.json
This is for simplified Chinese so the correct locale should be zh_CN. For traditional Chinese it should be something like zh_HK or zh_TW.
@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls May 20, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling e41cc24 on andyhu:patch-1 into 2a73ed4 on yargs:master.

coveralls commented May 20, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling e41cc24 on andyhu:patch-1 into 2a73ed4 on yargs:master.

@bcoe

This comment has been minimized.

Show comment
Hide comment
@bcoe

bcoe May 20, 2016

Member

@andyhu thank you. I have a concern though, this could break existing apps that were relying on the zh locale name; perhaps we should just have two locale files? zh.json and zh_CN.json?

Member

bcoe commented May 20, 2016

@andyhu thank you. I have a concern though, this could break existing apps that were relying on the zh locale name; perhaps we should just have two locale files? zh.json and zh_CN.json?

@andyhu

This comment has been minimized.

Show comment
Hide comment
@andyhu

andyhu May 20, 2016

Contributor

Hi @bcoe, I have the same concern. However I'm a Chinese native speaker and I've never used the locale string 'zh' since it's quite different between Traditional Chinese and Simplified Chinese. As Chinese we can understand both of them though. Or maybe we can change it until next major release after something like zh_TW is available?

Contributor

andyhu commented May 20, 2016

Hi @bcoe, I have the same concern. However I'm a Chinese native speaker and I've never used the locale string 'zh' since it's quite different between Traditional Chinese and Simplified Chinese. As Chinese we can understand both of them though. Or maybe we can change it until next major release after something like zh_TW is available?

@bcoe

This comment has been minimized.

Show comment
Hide comment
@bcoe

bcoe May 21, 2016

Member

@andyhu let's just keep zh.json and zh_CN.json and in the next major we'll drop zh.json.

Member

bcoe commented May 21, 2016

@andyhu let's just keep zh.json and zh_CN.json and in the next major we'll drop zh.json.

@andyhu

This comment has been minimized.

Show comment
Hide comment
@andyhu

andyhu May 21, 2016

Contributor

thanks @bcoe

Contributor

andyhu commented May 21, 2016

thanks @bcoe

@bcoe

This comment has been minimized.

Show comment
Hide comment
@bcoe

bcoe May 21, 2016

Member

@andyhu mind submitting a pull with both files, then I can give you credit for the work when I merge.

Member

bcoe commented May 21, 2016

@andyhu mind submitting a pull with both files, then I can give you credit for the work when I merge.

@maxrimue maxrimue referenced this pull request May 27, 2016

Closed

Version 5.0.0 #519

10 of 10 tasks complete

@maxrimue maxrimue added the 5.x label May 27, 2016

@bcoe

This comment has been minimized.

Show comment
Hide comment
@bcoe

bcoe Jun 1, 2016

Member

@andyhu thank you for the contribution \o/ closing in favor of #519 which keeps zh.json, and adds zh_CN.json.

Member

bcoe commented Jun 1, 2016

@andyhu thank you for the contribution \o/ closing in favor of #519 which keeps zh.json, and adds zh_CN.json.

@bcoe bcoe closed this Jun 1, 2016

@bcoe bcoe referenced this pull request Aug 7, 2016

Merged

fix: remove deprecated zh.json #578

@bcoe

This comment has been minimized.

Show comment
Hide comment
@bcoe

bcoe Aug 14, 2016

Member

@andyhu give this a spin, I've landed your changes finally:

npm cache clear; npm i yargs@next

Member

bcoe commented Aug 14, 2016

@andyhu give this a spin, I've landed your changes finally:

npm cache clear; npm i yargs@next

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment