Skip to content
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

What about links? #1

Closed
aviflax opened this issue Jul 21, 2015 · 6 comments
Closed

What about links? #1

aviflax opened this issue Jul 21, 2015 · 6 comments

Comments

@aviflax
Copy link

aviflax commented Jul 21, 2015

This lib looks useful, thanks!

Unfortunately I can’t use it as-is because the list of time zones seems to omit “links” such as US/Eastern; there appear to be 140 of these according to this page and I definitely have my client apps providing these values to my server and I want them to pass validation.

Could you share whether there’s any particular reason that the list omits these “links”?

Thanks!

@jhvst
Copy link
Owner

jhvst commented Jul 21, 2015

Hi and thanks for submitting the issue!

I generated the tables from CSV files downloaded from a third party website, which seemingly did not include the links. I did not have any reference library to build from, so the absence never came up.

Testing the underlying Offset code in Go Playground shows that the link locations can be parsed with Go's time package. Therefore, I agree that the links should be added.

I wouldn't mind a pull request, but I'll try to be productive enough to add the missing locations by myself during this week.

@aviflax
Copy link
Author

aviflax commented Jul 21, 2015

Cool. I don’t think I’ll be able to submit a contribution, sorry. But thanks!

@jhvst
Copy link
Owner

jhvst commented Jul 22, 2015

Hi again,

It looks like there would be 88 location additions, including the following for US:

US/Alaska
US/Aleutian
US/Arizona
US/Central
US/East-Indiana
US/Eastern
US/Hawaii
US/Indiana-Starke
US/Michigan
US/Mountain
US/Pacific
US/Pacific-New
US/Samoa

Do you notice anything missing?

I did remove few backwards compatible entries such as these, since most of them aren't really locations, which makes time inaccurate. The updated location list array will be made from the following file: https://github.com/9uuso/timezone/blob/dev/2015e/loc

Can you check are the backwards compatible timezones listed in your frontend library? If they are, I have to figure out how to deal with the backward compatible zones.

If these changes satisfy you, I'll generate the Go code and merge the changes.

@aviflax
Copy link
Author

aviflax commented Jul 22, 2015

Honestly I think rather than try to base the list on any given client behavior, we should stick with the canonical IANA list. I’m hoping that the Wikipedia page is an accurate representation of that list.

I extracted a list of all the values from that page. I count 538 values in that list, whereas the file you linked in your dev branch has only 504.

My preference would be for the lib to include all 538 values listed in the Wikipedia page.

@jhvst
Copy link
Owner

jhvst commented Jul 24, 2015

I downloaded the latest package from IANA (version 2015e -- Wikipedia has 2014b) and updated the locations to include timezone links. The new array is 584 elements long and includes the timezones that you were missing.

@jhvst jhvst closed this as completed Jul 24, 2015
@aviflax
Copy link
Author

aviflax commented Jul 24, 2015 via email

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

No branches or pull requests

2 participants