Skip to content
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 TimeClip, MakeTime, MakeDay, and MakeDate #4

Closed
wants to merge 7 commits into from

Conversation

mathiasbynens
Copy link
Contributor

No description provided.

assert.strictEqual(isNaN(abstractOps.TimeClip(-Infinity)), isNaN(NaN));
assert.strictEqual(isNaN(abstractOps.TimeClip(+Infinity)), isNaN(NaN));
assert.strictEqual(isNaN(abstractOps.TimeClip(-8.64e15 - 1)), isNaN(NaN));
assert.strictEqual(isNaN(abstractOps.TimeClip(+8.64e15 + 1)), isNaN(NaN));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I’m using isNaN here because Mocha would throw AssertionError: "NaN" === "NaN" otherwise. Maybe there’s a better way?

Copy link
Owner

Choose a reason for hiding this comment

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

This is kind of a strange way to do it; I would just do assert(Number.isNaN(...))

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, duh! Patch updated.

(Still, this feels like it’s a bug in Mocha.)

@mathiasbynens mathiasbynens changed the title Add TimeClip Add TimeClip and MakeTime Aug 8, 2014
@mathiasbynens mathiasbynens changed the title Add TimeClip and MakeTime Add TimeClip, MakeTime, MakeDay, and MakeDate Aug 8, 2014
@@ -242,9 +242,39 @@ exports.ToLength = function (argument) {
return min(len, Math.pow(2, 53) - 1);
};

// https://people.mozilla.org/~jorendorff/es6-draft.html#sec-hours-minutes-second-and-milliseconds
const HoursPerDay = 24;
Copy link
Owner

Choose a reason for hiding this comment

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

Spacing is off

@mathiasbynens
Copy link
Contributor Author

Thanks for the review. Patch amended.

@domenic
Copy link
Owner

domenic commented Aug 8, 2014

Thanks, squashed and merged as 07f167b; 1.8.0 to be published shortly.

@domenic domenic closed this Aug 10, 2014
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.

None yet

2 participants