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

support for millisecond-precise time inputs for jday #38

Merged
merged 2 commits into from
Dec 22, 2017

Conversation

davidcalhoun
Copy link
Contributor

Based on #32, with some extra testing goodies this time. :)

@coveralls
Copy link

coveralls commented Nov 21, 2017

Coverage Status

Coverage increased (+0.06%) to 83.869% when pulling afc0d75 on davidcalhoun:master into c6bc96c on shashwatak:develop.

@ezze
Copy link
Collaborator

ezze commented Nov 21, 2017

@davidcalhoun, thanks a lot for your new pull request!

Are you sure that your new tests will pass when now.getUTCMilliseconds() returns 0?

I also want to ask you to look at new invjday function I ported from original Python library and check whether it's possible to add milliseconds support there.

@ezze
Copy link
Collaborator

ezze commented Nov 22, 2017

Probably it's better to use predefined dates for testing instead of new Date().

@davidcalhoun
Copy link
Contributor Author

Thanks for having a look. Will try to address your comments later this evening.

@ezze
Copy link
Collaborator

ezze commented Nov 26, 2017

@davidcalhoun, have you any chance to look at this soon? Let me know if you still want to finish this task by yourself, or I'll try to continue it with my own efforts.

@davidcalhoun
Copy link
Contributor Author

Ack, sorry I didn't make time for this :(. If you want to have a shot at it and have more time than me, go for it. :) . I won't mind if you close this.

@ezze ezze merged commit afc0d75 into shashwatak:develop Dec 22, 2017
@ezze
Copy link
Collaborator

ezze commented Dec 22, 2017

@davidcalhoun, thanks a lot for your contribution again. I merged this with minor test fixes.

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

3 participants