Since until#259
Conversation
…Tennebekk/temporal into add-since-until-yearmonth
nekevss
left a comment
nekevss
left a comment
There was a problem hiding this comment.
Looks great! All the extra tests looks great!
I had one thing I noticed and then a general thought/nit (this one isn't necessarily blocking though). Although, it does look like there are some clippy lints that need to be fixed as well to be able to merge.
| // 15. Let duration be CombineDateAndTimeDuration(yearsMonthsDifference, 0). | ||
| let mut duration = NormalizedDurationRecord::from_date_duration(*result.date())?; | ||
| // 16. If settings.[[SmallestUnit]] is not month or settings.[[RoundingIncrement]] ≠ 1, then | ||
| if settings.smallest_unit != Some(TemporalUnit::Month) |
There was a problem hiding this comment.
issue: I believe this should be using the resolved options in resolved.
| // 1. NOTE: The following steps read options and perform independent validation in alphabetical order. | ||
| // 2. Let largestUnit be ? GetTemporalUnitValuedOption(options, "largestUnit", unitGroup, auto). | ||
| // Week and day is the only use case where any of the units are not allowed. | ||
| if dissallow_week_and_day { |
There was a problem hiding this comment.
thought: is there anything preventing moving this into PlainYearMonth::diff vs. where it's currently at in ResolvedRoundingOptions::from_diff_settings.
This only affects the one code path from PlainYearMonth, I think, so maybe this could work in PlainYearMonth::diff. This would then avoid the check occurring for the other callers.
…resolved from settings in diff and moved logic from options.rs from_diff_setting into diff
4 participants
fixes #142
Worked with @HenrikTennebekk and @Magnus-Fjeldstad
Implementation for since and until in PlainYearMonth, has not yet been tested with boa test262