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

Enable Time::Location loader on win32 #6363

Merged
merged 5 commits into from Aug 31, 2018

Conversation

straight-shoota
Copy link
Member

@straight-shoota straight-shoota commented Jul 10, 2018

With #5623 merged, the location loader methods can finally use the File API on win32.

I also fixed a few missing with_zoneinfo. The spec with ENV["TZ"] = "" doesn't work on win32 because empty string is identified as nil (see #6333 (comment)).

With File support for win32, the POSIX implementations work on win32 as
well.
@straight-shoota straight-shoota changed the title Enable Time::Location features on win32 Enable Time::Location loader on win32 Jul 10, 2018
Copy link
Member

@asterite asterite left a comment

Choose a reason for hiding this comment

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

I didn't try this on Windows, but I trust @RX14 and @straight-shoota if they tested it.

Copy link
Contributor

@ysbaddaden ysbaddaden left a comment

Choose a reason for hiding this comment

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

Looks good. Feel free to merge when ready (or is it?).

@RX14 RX14 merged commit d4605d7 into crystal-lang:master Aug 31, 2018
@RX14 RX14 added this to the 0.27.0 milestone Aug 31, 2018
@straight-shoota straight-shoota deleted the jm/fix/time-enable-win branch September 3, 2018 18:59
ezrast pushed a commit to ezrast/crystal that referenced this pull request Oct 2, 2018
* Enable Time::Location loader on win32

With File support for win32, the POSIX implementations work on win32 as
well.

* Add missing with_zoneinfo in Time spec

* Fix Time::Location.load_local with TZ and ZONEINFO

* fixup! Enable Time::Location loader on win32

Replace File.expand_path with File.join to work on win32

* fixup! Add missing with_zoneinfo in Time spec
@straight-shoota straight-shoota added this to Todo in Windows via automation Jan 8, 2019
@straight-shoota straight-shoota moved this from Todo to Stdlib in Windows Jan 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Windows
  
Stdlib
Development

Successfully merging this pull request may close these issues.

None yet

4 participants