Skip to content

Update time zone abbreviations#104

Closed
matthewbadeau wants to merge 3 commits intofelixge:masterfrom
matthewbadeau:patch-1
Closed

Update time zone abbreviations#104
matthewbadeau wants to merge 3 commits intofelixge:masterfrom
matthewbadeau:patch-1

Conversation

@matthewbadeau
Copy link

@matthewbadeau matthewbadeau commented Jun 30, 2020

Uses list of time zone abbreviations. https://en.wikipedia.org/wiki/List_of_time_zone_abbreviations

I expanded the regular expression to use all time zone abbreviations.

As an example AEST time zones are being clipped to just EST. This change fixes that problem. It also fixes unreported problems with other time zones. According to Wikipedia the longest time zone is five characters.

@chase-manning
Copy link
Collaborator

@matthewbadeau Hi Matthew, thanks so much for the PR!!
Sorry this was not merged sooner, the repo was not being actively maintained at the time.
I would love to merge this PR now, but it has a merge conflict.
Would you be able to please resolve this merge conflict so I can merge it?

@matthewbadeau
Copy link
Author

@chase-manning Hi there! I went nuclear and just pulled the latest and added the changes I wanted to make.

The tests aren't passing but I think it's because the time zone offset is not empty when the tests are run. The test replaces GMT+0000 to UTC but expects GMT. I might be misunderstanding it though. If I have time, I'll try to make another PR with changes to the tests.

@matthewbadeau
Copy link
Author

Sorry for the multiple comments.

The tests are currently passing but it still doesn't fix the issue where 4 character time zones are not being displayed. For example, running LC_ALL="en_AU" TZ="Australia/Hobart" npm test fails. Running LC_ALL="en_US" TZ="Asia/Tokyo" npm test passes.

Before merging I will investigate more.

@mikegreiling
Copy link
Contributor

@chase-manning is there anything that can be done to move this PR forward?

@chase-manning
Copy link
Collaborator

@chase-manning is there anything that can be done to move this PR forward?

@mikegreiling It currently has a merge conflict, once that is fixed we can merge it 😄

@milohax milohax mentioned this pull request Sep 6, 2021
@mikegreiling
Copy link
Contributor

@chase-manning it looks like @milohax created a MR to address the conflicts with #165

@chase-manning
Copy link
Collaborator

Closing as this was completed in a seperate PR

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.

3 participants