Skip to content
This repository has been archived by the owner on Nov 29, 2018. It is now read-only.

Update culture names list #101

Closed
wants to merge 1 commit into from
Closed

Update culture names list #101

wants to merge 1 commit into from

Conversation

hishamco
Copy link
Contributor

@hishamco hishamco commented Oct 7, 2015

I create this PR based on issue #98

@hishamco
Copy link
Contributor Author

hishamco commented Oct 7, 2015

/cc @Eilon @kirthik

@Eilon Eilon added this to the 1.0.0-rc1 milestone Oct 7, 2015
@hishamco hishamco mentioned this pull request Oct 8, 2015
Closed
@yukozh yukozh mentioned this pull request Oct 8, 2015
Closed
yukozh
yukozh reviewed Oct 8, 2015
View reviewed changes
src/Microsoft.Extensions.Globalization.CultureInfoCache/CultureInfoList.cs
@@ -431,6 +820,8 @@ public static partial class CultureInfoCache
"zh",
"zh-CN",
"zh-Hans",
"zh-Hans-HK",
Copy link

Choose a reason for hiding this comment

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

It should be zh-Hant-HK & MO, see #104

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As I mentioned before in #103 this generated by tool

@kirthik
Copy link
Contributor

kirthik commented Oct 9, 2015

Updating CultureInfoList with just one CultureInfo may not be the right solution. Closing this one since we already have another PR - #104 and a related issue for it - #98.

@kirthik kirthik closed this Oct 9, 2015
@hishamco
Copy link
Contributor Author

@kirthik i'm still surprised why this PR has been closed, while it fix issue #98 after @DamianEdwards confirm. Regarding PR #104 i'm not sure if @kagamine added the cultures manually or using the CultureInfoGenerator command, furthermore if you look to the updated list may you will understand that more than 400+ cultures have been added

@hishamco
Copy link
Contributor Author

@Eilon is this closed by mistake?!!

@yukozh
Copy link

yukozh commented Oct 10, 2015

I think she would close the PR #104 ...

@hishamco
Copy link
Contributor Author

That's what I expect 😄

@yukozh
Copy link

yukozh commented Oct 10, 2015

Why you care it so much?

@hishamco
Copy link
Contributor Author

Because the PR sent after @DamianEdwards confirmation, furthermore the culture that you come up with is interesting and may we find a smart solution to generate all the culture cross platform
thanks @kagamine

@Eilon Eilon reopened this Oct 10, 2015
@Eilon
Copy link
Member

Eilon commented Oct 10, 2015

We will discuss these next week and get back to you.

@hishamco
Copy link
Contributor Author

Thanks @Eilon

@Eilon
Copy link
Member

Eilon commented Oct 14, 2015

@kirthik given our discussion today, I believe this is ok to merge.

@kirthik
Copy link
Contributor

kirthik commented Oct 14, 2015

@Eilon : Based on the discussion in #111, plan is to remove KnownCultureNames. Then this PR will not be relevant right?

@Eilon
Copy link
Member

Eilon commented Oct 14, 2015

@kirthik oh yeah I was confusing the cache and the known list.

@kirthik
Copy link
Contributor

kirthik commented Oct 15, 2015

Closing this PR since it will no longer be relevant after #111 is implemented.

@kirthik kirthik closed this Oct 15, 2015
@hishamco
Copy link
Contributor Author

@kirthik Is this imply to close issue #98?!!

@Eilon
Copy link
Member

Eilon commented Oct 15, 2015

Yes, I closed #98.

@hishamco hishamco mentioned this pull request Oct 21, 2015
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@kirthik kirthik

Labels
None yet
Projects
None yet
Milestone
1.0.0-rc1
Development

Successfully merging this pull request may close these issues.
5 participants
@hishamco @kirthik @yukozh @Eilon @dnfclas