Adjustments to rounding options in difference() and round() methods #908
Assignees
Labels
documentation
Additions to documentation
non-prod-polyfill
THIS POLYFILL IS NOT FOR PRODUCTION USE!
spec-text
Specification text involved
Milestone
Comments
This was referenced Sep 15, 2020
Merged
ptomato
added
documentation
|
Both these proposed changes sound good to me. Should this be in the design-decisions milestone or the current Stable Proposal milestone? I admit I don't fully understand the difference between the two. |
|
The design-decisions milestone is what I'm currently using as the agenda for our meetings, it needs to be emptied out ASAP. I think these are probably uncontroversial enough that we don't need to spend meeting time discussing them. |
|
BTW, my strong preference for (1) above would be that Instant should support |
ptomato
added a commit
that referenced
this issue
Oct 13, 2020
The rationale for not allowing this was that the starting point of the hour was not well-defined, due to Instant not having a time zone. However, this applies to minutes as well, and so hours and minutes should either be both allowed or both disallowed. We decide to allow them for convenience. See: #908
ptomato
added a commit
that referenced
this issue
Oct 13, 2020
We don't have Date.round because calendar rounding needs a reference point, but in Date.difference we have a reference point (`this`). See: #908
ptomato
added a commit
that referenced
this issue
Oct 13, 2020
For the same reason as we can add it to Date.difference, `this` is used as the reference point. See: #908
ptomato
added a commit
that referenced
this issue
Oct 13, 2020
We don't have Date.round because calendar rounding needs a reference point, but in Date.difference we have a reference point (`this`). See: #908
ptomato
added a commit
that referenced
this issue
Oct 13, 2020
For the same reason as we can add it to Date.difference, `this` is used as the reference point. See: #908
Ms2ger
pushed a commit
that referenced
this issue
Oct 14, 2020
The rationale for not allowing this was that the starting point of the hour was not well-defined, due to Instant not having a time zone. However, this applies to minutes as well, and so hours and minutes should either be both allowed or both disallowed. We decide to allow them for convenience. See: #908
Ms2ger
pushed a commit
that referenced
this issue
Oct 14, 2020
We don't have Date.round because calendar rounding needs a reference point, but in Date.difference we have a reference point (`this`). See: #908
Ms2ger
pushed a commit
that referenced
this issue
Oct 14, 2020
For the same reason as we can add it to Date.difference, `this` is used as the reference point. See: #908
|
Both of these were addressed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Follow-up from #827.
Two questions I'd like to open that arose after implementing the rounding proposal:
absolute.round({ smallestUnit })allows"minutes"forsmallestUnitbut not"hours". The reason for this was that hours are not defined in an Absolute and would need a time zone-aware reference point. But this seems inconsistent. If hours are not well-defined, then neither are minutes. Currently rounding to minutes uses the Unix epoch as 0 minutes, and you can get the same effect as rounding to 1 hour by specifying minutes with an increment of 60. I think we should either have both hours and minutes, or neither.We don't have rounding in
Temporal.Date.prototype.differenceandTemporal.YearMonth.prototype.difference, but there's nothing actually preventing this. We decided not to do date rounding in the round() method because you need a reference point, but in difference() you have a reference point.The text was updated successfully, but these errors were encountered: