Instant.toSeconds should return int #62
Conversation
This is generally safe (for now), and should be included to be consistent with LocalDateTime.toSeconds, which is already an int. Also add some doc.
| * | ||
| * This uses a checked conversion, it will overflow and raise if the year is after 2038. | ||
| */ | ||
| def toSeconds: Int = Math.toIntExact(d.toEpochSecond(utc_)) |
There was a problem hiding this comment.
We can use LocalDateTime, Instant to present sometime in the future, so this is not actually safe? I guess to make things more consistent, we should return Long here instead of return Int everywhere else.
There was a problem hiding this comment.
Yes it's not totally safe because even in 2024 we could theoretically construct a LocalDateTime or Instant that corresponds to year 2038. That's why it's a checked cast for the LocalDateTime and Instant extension methods but unchecked for nowSeconds
It's a little unclean but working with ints when ints suffice is worth it, for now imo. This won't come up as an issue in lila until 2036 or later.
There was a problem hiding this comment.
I think I just recently changed the other method to Int, yes we could undo them and return long. But do we really need to? Idk
|
I think it's ok for now. Hi future me! Hope you're doing great. |
This is generally safe (for now), and should be included to be consistent with LocalDateTime.toSeconds, which is already an int.
Also add some doc.