-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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 Time.utc
#5321
Add Time.utc
#5321
Conversation
@@ -19,7 +19,10 @@ require "crystal/system/time" | |||
# Time.new(2016, 2, 15) # => 2016-02-15 00:00:00 | |||
# | |||
# # Specifying a time | |||
# Time.new(2016, 2, 15, 10, 20, 30) # => 2016-02-15 10:20:30 UTC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doc line was actually wrong, Time.new(2016, 2, 15, 10, 20, 30)
currently returns a time of kind Unspecified
, which does not have a UTC
timezone.
3a9cb40
to
1de9188
Compare
Creating a UTC time instance is fairly commmon and should not bother with internals instead have a simple API for this. This fits nicely with already existing `Time.utc_now`.
1de9188
to
3006b61
Compare
HTTP.parse_time("Sun, 06 Nov 1994 08:49:37 GMT").should eq(time) | ||
end | ||
|
||
it "parses RFC 1123 without day name" do | ||
time = Time.new(1994, 11, 6, 8, 49, 37) | ||
time = Time.utc(1994, 11, 6, 8, 49, 37) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should not be UTC (following line check it's a GMT time)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this is completely valid.
- HTTP dates represented in the RFC 1123 format are usually marked as
GMT
but are really UTC times, because the standard doesn't recognizeUTC
. This is also how the Crystal implementation works. - This method is used to create a time instance refering to a specific point in time. For the equality check, the individual time zones do not really matter, because the values are compared based on UTC anyway. It has just to be assured that both, the RFC 1123 string representation and the local comparison date refere to the same point in time. And that, they do.
- Apart from that, there is not really a difference between dates in GMT and UTC zones at all in Crystal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My bad, forgot GMT was a timezone.
Thank you @straight-shoota for your contribution! I like the Changes LGTM. |
I'm not sure about it, is using |
It depends, if you're writing a lot of time-related calculations and generations, typing that every single time can result tedious. Use your clipboard buffer to hold that single snippet or adding that as snippet of your editor is also tedious. There are several examples were those shortcuts were added to the codebase that reduced typing and core decided that was a win for the developers without bending the Crystal core rule of no aliases. But again, it definitely depends on usage. 🤷♂️ |
@RX14 Same question can be applied to many other instances where shorthand notation is just preferred because of brevity/speed/semantic/etc... |
@RX14 |
53a7afe
to
89d89c6
Compare
Hello @mverzilli and @bcardiff, can you guys weight in and comment/approve this change? Thank you. |
Cool stuff, thanks @straight-shoota! |
It is fairly common to create a
Time
instance in UTC. Until now, to create such an instance, you'd have to useTime.new
withkind: Time::Kind::Utc
.This PR adds a shortcut for this specific task, named
Time.utc
which accepts the same arguments asTime#now
with date and time fields or with seconds and nanoseconds and sets the kind toUtc
. It fit's nicely with the already existingTime.utc_now
.This makes it less verbose to use UTC times for developers and it will be easier to replace the current
Time#kind
with a proper time zone implementation (see #2490).As a minor edit I also replaced direct comparisons of
Time#kind
withTime::Kind::Utc
withTime.utc?
which serves the same purpose.