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

Add ToTimeInDefaultLocation/E #80

Closed
wants to merge 2 commits into from
Closed

Add ToTimeInDefaultLocation/E #80

wants to merge 2 commits into from

Conversation

bep
Copy link
Collaborator

@bep bep commented May 30, 2019

No description provided.

@bep bep force-pushed the date-with-tz branch 3 times, most recently from 62d766f to 2e435db Compare May 30, 2019 18:39
@bep bep changed the title Add ToTimeInDefaultLocation/E Work in progress: Add ToTimeInDefaultLocation/E May 30, 2019
@bep bep force-pushed the date-with-tz branch 2 times, most recently from 2172ccb to 69c1d41 Compare May 31, 2019 08:49
@bep bep changed the title Work in progress: Add ToTimeInDefaultLocation/E Add ToTimeInDefaultLocation/E May 31, 2019
Go's time parsing uses UTC when the format doesn't have a tiemzone, and
has even weirder behavior when it has a zone name but no numeric offset.
A caller to `cast.ToTime` won't know if the returned time was explicitly
in UTC, or defaulted there, so the caller cannot fix it. These new
functions allow a user to supply a different timezone to default to,
with nil using the local zone.
@bep bep force-pushed the date-with-tz branch 6 times, most recently from 0881fcb to dc790bf Compare May 31, 2019 10:10
@bep bep changed the title Add ToTimeInDefaultLocation/E WORK IN PROGRESS: Add ToTimeInDefaultLocation/E May 31, 2019
@bep bep force-pushed the date-with-tz branch 9 times, most recently from f5b8561 to 089f9f4 Compare May 31, 2019 12:38
This commit adjusts the previous commit re. the new `ToTimeInDefaultLocationE` and related functions.

The functional change is that we don't default to any location if not provided from the caller.
This is in line with how `ToTime` worked before we started this, and even if the default behaviour may look weird in some cases, it will not break anything.

Most applications will want to use the new *InDefaultLocation functions and decide which default location to use:

```go
loc := time.Local
if config.Location != "" {
    loc = time.LoadLocation(config.Location)
}

t, err := StringToDateInDefaultLocation("2019-01-01", loc)

```

This commit also configure Travis to test on OSX and Windows in addition to Linux.
@bep bep changed the title WORK IN PROGRESS: Add ToTimeInDefaultLocation/E Add ToTimeInDefaultLocation/E May 31, 2019
@bep bep requested a review from spf13 May 31, 2019 15:23
@bep
Copy link
Collaborator Author

bep commented Jun 1, 2019

The context for this PR is described in #23 from @heewa

I can just repeat here that the reason we don't use time.ParseInLocation for this is that it behaves oddly (which is a nicer word for wrong) in some cases.

One example would be that time.ParseInLocation("Mon Jan _2 15:04:05 MST 2006", "Fri Jan 1 03:01:00 EST 2016", iranLocation) would produce 2016-01-01 03:01:00 +0000 EST. The offset is obviously not correct for any of them. With the implementation in this PR that example would produce 2016-01-01 03:01:00 +0330 +0330 -- with the offset of Iran.

@moorereason
Copy link
Contributor

This can be closed since e4dda5f.

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.

None yet

3 participants