-
Notifications
You must be signed in to change notification settings - Fork 17
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
Utc #54
Conversation
src/fromTime.js
Outdated
@@ -0,0 +1 @@ | |||
export default time => new Date(time) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we handle Invalid Date
here?
|
||
const clone = date => new Date(date.getTime()) | ||
|
||
module.exports = { | ||
of, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we need to add these new functions to the docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup, but I expect a few iterations, befor we settle on how the new api should be
src/of.js
Outdated
@@ -0,0 +1,12 @@ | |||
const isNotNil = x => x !== undefined && x !== null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
!= null
is much nicer imo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Im not fuzzed either way
src/fromTime.js
Outdated
@@ -0,0 +1 @@ | |||
export default time => new Date(time) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder should we call this function fromUnixTime
and rename unixTime
to toUnixTime
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would be ok with that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
though unix time is in seconds and javascript getTime
is in millies
I think this is ready for mere. It does not include functions for translating between timezones, but I would rather add that on later. It is a breaking change. |
Yep, tidy up the conflicts and we're good to go. |
all functions are now using the utc version of date methods add of and fromTime functions for creation of utc dates fixed test coverage fixed testBuild addded npm run flow to curcle.yml add isInvalid check to fromTime added translation added docs for of, and fromTime
Done |
DO NOT MERGE!