Skip to content

[Temporal] Remove test/staging/Temporal/v8/calendar-date-until.js#4817

Merged
ptomato merged 5 commits intotc39:mainfrom
catamorphism:v8-staging-calendar-date-until
Jan 19, 2026
Merged

[Temporal] Remove test/staging/Temporal/v8/calendar-date-until.js#4817
ptomato merged 5 commits intotc39:mainfrom
catamorphism:v8-staging-calendar-date-until

Conversation

@catamorphism
Copy link
Contributor

And move functionality into one existing test and one new test.

@catamorphism catamorphism requested review from a team as code owners January 7, 2026 01:47
Comment on lines +25 to +30
for (const largestUnit of units) {
for (const smallestUnit of units) {
assert.throws(RangeError, () => from.until(to, { largestUnit, smallestUnit }),
`Can't use ${largestUnit} and ${smallestUnit} as largestUnit and smallestUnit for PlainDate`);
assert.throws(RangeError, () => from.until(to, { smallestUnit, largestUnit }),
`Can't use ${smallestUnit} and ${largestUnit} as largestUnit and smallestUnit for PlainDate`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't covered yet, but I don't think we need the two calls in the loop (unless you meant to do something different in the second one?)

If we keep it, please add the parallel test for since

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the parallel test for since in 40ad13f. The second call was supposed to swap smallestUnit and largestUnit -- I fixed it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still feel like I'm missing something. You're looping over the same units array twice, so you're getting each combination already.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yeah, you're right. Removed the second call.

Copy link
Contributor

@ptomato ptomato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@catamorphism catamorphism enabled auto-merge (rebase) January 19, 2026 21:09
@ptomato ptomato disabled auto-merge January 19, 2026 21:32
@ptomato ptomato enabled auto-merge (squash) January 19, 2026 21:33
@ptomato ptomato force-pushed the v8-staging-calendar-date-until branch from 711d5e7 to c6b08c3 Compare January 19, 2026 21:33
@ptomato ptomato merged commit c9aa612 into tc39:main Jan 19, 2026
14 checks passed
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.

3 participants