-
Notifications
You must be signed in to change notification settings - Fork 156
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
Switch from moment-timezone to luxon for smaller bundle #202
Switch from moment-timezone to luxon for smaller bundle #202
Conversation
d6a2d52
to
b323595
Compare
@andytson thanks for your contribution! LGTM but I'll in depth review ASAP. I'm also thinking about releasing this as a new major version (3.0). |
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.
Thanks again! Added some comments.
61b5563
to
3d42662
Compare
Luxon uses Intl APIs for IANA timezones rather than bundling them Note: CronDate implemented to support the same types the typescript definitions define, rather than moment's additional array/object timestamp support that could have potentially been used before.
for ability to switch to another date library later
eb75cdd
to
7a9453f
Compare
Removing unnecessary bad format handling
7a9453f
to
f8f68a7
Compare
@andytson https://github.com/harrisiirak/cron-parser/releases/tag/3.0.0 and I also published it to npm. Thank for your hard work and the contribution! |
Luxon uses Intl APIs for IANA timezones rather than bundling them
Note: CronDate implemented to support the same types the typescript definitions define, rather than moment's additional array/object timestamp support that could have potentially been used before. - I can add that if necessary
Fixes #195