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

feat: add luxon date adapter #52

Merged
merged 4 commits into from
Oct 9, 2024
Merged

Conversation

smn-3-14
Copy link
Contributor

@smn-3-14 smn-3-14 commented Sep 26, 2024

I have completed the Luxon date-adapter in reference to this pull request: mattlewis92/angular-calendar#1709.

It would be appreciated if we could get this merged and released. I am also prepared to proceed with adding Luxon support to the main repository.

Thank you!

package-lock.json Outdated Show resolved Hide resolved
Copy link
Owner

@mattlewis92 mattlewis92 left a comment

Choose a reason for hiding this comment

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

This is really great, thank you!! I have never used luxon before so I had a few questions, no worries if they're not applicable (and I guess if the test suite passes, it's all ok)

@smn-3-14
Copy link
Contributor Author

smn-3-14 commented Oct 2, 2024

My latest changes should cover all your suggestions. We probably have to update the package-lock separately. Would you mind taking care of this so I can rebase?
Thanks for your time :)

@mattlewis92
Copy link
Owner

My latest changes should cover all your suggestions. We probably have to update the package-lock separately. Would you mind taking care of this so I can rebase? Thanks for your time :)

Ah I see, please rebase on main, run corepack enable to install pnpm, and then pnpm i to install deps (switching to pnpm will be more future proof than relying on a bundle npm version with node)

Copy link
Owner

@mattlewis92 mattlewis92 left a comment

Choose a reason for hiding this comment

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

LGTM! Will merge and release once the CI is fixed (I tried to do it myself but github blocked me)

@smn-3-14
Copy link
Contributor Author

smn-3-14 commented Oct 4, 2024

Alright, we should be good to go now!

@mattlewis92
Copy link
Owner

It looks like the tests are failing, please can you commit the updated snapshots? https://github.com/mattlewis92/calendar-utils/actions/runs/11175436044/job/31217858636?pr=52

Copy link

codecov bot commented Oct 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (9728b25) to head (a5b1502).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main       #52   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            5         6    +1     
  Lines          508       559   +51     
  Branches        95        95           
=========================================
+ Hits           508       559   +51     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mattlewis92 mattlewis92 merged commit ecf544d into mattlewis92:main Oct 9, 2024
3 checks passed
@mattlewis92
Copy link
Owner

Published as calendar-utils@0.11.0, thanks for your contribution! 🥳

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.

2 participants