Skip to content

Expose IANA Time Zone territories to library consumers#64

Merged
mattjohnsonpint merged 10 commits intomattjohnsonpint:mainfrom
daiplusplus:dev/2020-03-18-info
Sep 3, 2022
Merged

Expose IANA Time Zone territories to library consumers#64
mattjohnsonpint merged 10 commits intomattjohnsonpint:mainfrom
daiplusplus:dev/2020-03-18-info

Conversation

@daiplusplus
Copy link
Contributor

@daiplusplus daiplusplus commented Mar 19, 2020

This PR changes the library to expose Territory name information from TimeZoneConverter.Data.Mapping.csv.gz as a separate dictionary property on TZConvert.

I've also added an .editorconfig file and unit tests (which pass, though one of the other existing POSIX tests fails, it's not relevant to this PR).

I used the existing coding style and conventions (though I did align some statements and add braces to others for the sake of readability). I note that I'm not entirely comfortable with continuing to use mutable collections on TZConvert. If you're interested I'd love to submit a PR that updates the project to use immutable collections.

Update: Removing "001":

I realised that zones were being added for territory "001" - I've updated the PR to exclude those (and added a test to ensure "001" isn't listed). I don't know where "001" comes from - can you explain why it exists and what it means? Thanks!

@mattjohnsonpint
Copy link
Owner

@Jehoel - Sorry it's taking me so long to get to reviewing this PR. I definitely appreciate your interest in helping out here!

I'm not sure if I fully agree with the form of this API, but I will take a more detailed look and get back to you soon (hopefully!) 😉

With regard to 001, that identifies the CLDR "Golden Zone", which is used as the primary mapping for the Windows-to-IANA direction when a country code is not specified. You can see it in the CLDR windowsZones.xml mapping file. If you like, you can also read more in the Unicode TR35 (LDML) datetime spec (search for golden zone and/or "001").

@mattjohnsonpint mattjohnsonpint self-assigned this Jan 22, 2021
Base automatically changed from master to main January 22, 2021 21:43
@daiplusplus
Copy link
Contributor Author

@mattjohnsonpint Ping?

@mattjohnsonpint
Copy link
Owner

Reviving this. I brought it up to the current code base and made some more changes. A couple of more things to do to finish before merging.

@mattjohnsonpint
Copy link
Owner

I reworked the implementation quite a bit, but this gets the same job done you were originally proposing. Thanks again!

@mattjohnsonpint mattjohnsonpint merged commit 303f3f5 into mattjohnsonpint:main Sep 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants