Skip to content

add docs for date module #61

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

Merged
merged 11 commits into from
Feb 23, 2023
Merged

add docs for date module #61

merged 11 commits into from
Feb 23, 2023

Conversation

dkirchhof
Copy link
Contributor

No description provided.

@zth
Copy link
Collaborator

zth commented Feb 20, 2023

@dkirchhof fantastic work! 👏

Let me know when you want me to take a look at it.

@zth zth mentioned this pull request Feb 20, 2023
49 tasks
@dkirchhof
Copy link
Contributor Author

@zth Ready to go!

JS dates are a pain. I'm curious about the temporal api.

@dkirchhof dkirchhof marked this pull request as ready for review February 20, 2023 19:16
Copy link
Collaborator

@zth zth left a comment

Choose a reason for hiding this comment

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

Great work, really solid! I have a few comments, but they would be mostly icing on the cake, this is already really great.

@dkirchhof
Copy link
Contributor Author

Thanks for the quick feedback. I will continue tomorrow ;)

@zth
Copy link
Collaborator

zth commented Feb 20, 2023

Thanks for the quick feedback. I will continue tomorrow ;)

Thank you for your contributions, it's great!

@dkirchhof
Copy link
Contributor Author

Should we mention that all the examples depend on the local time zone?
I was using export TZ='Europe/London' && node test/TempTests.mjs so the results of the examples wouldn't confuse anyone (my local time zone is MEZ).

@zth
Copy link
Collaborator

zth commented Feb 21, 2023

Should we mention that all the examples depend on the local time zone? I was using export TZ='Europe/London' && node test/TempTests.mjs so the results of the examples wouldn't confuse anyone (my local time zone is MEZ).

That's a good idea for sure, that can be slightly confusing with dates. I guess we'll need to add a "disclaimer" to all examples?

@dkirchhof
Copy link
Contributor Author

Changed the docs about epoch, added some more examples and added the disclaimer about results according to the local time zone.

@zth
Copy link
Collaborator

zth commented Feb 22, 2023

Changed the docs about epoch, added some more examples and added the disclaimer about results according to the local time zone.

This is looking great to me. You feel done with it?

@dkirchhof
Copy link
Contributor Author

Changed the docs about epoch, added some more examples and added the disclaimer about results according to the local time zone.

This is looking great to me. You feel done with it?

I would say so. Otherwise someone else could open a new PR for improvements?!

@zth
Copy link
Collaborator

zth commented Feb 23, 2023

Looking great to me, thank you!

@zth zth merged commit 64f449d into rescript-lang:main Feb 23, 2023
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