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

fix: correctly parse weekday ranges that include sunday #283

Merged
merged 3 commits into from
Mar 30, 2023

Conversation

sky0hunter
Copy link
Contributor

@sky0hunter sky0hunter commented Mar 14, 2023

This should solve the issue with parsing ranges like 6-7 for the weekday.

The tests are passing and with that small post processing of the weekdays element ( modulo + set -> remove duplicates and still have 0-6 ) it should do the exact same thing as before.

It should fix #282

@P4sca1
Copy link
Owner

P4sca1 commented Mar 14, 2023

Thank you for the PR. I will look into it within the next days. I think the problem still persists when using aliases, e.g. sat-sun, right?

@sky0hunter
Copy link
Contributor Author

I've added another commit which changes the sun alias to 7, so now these kind of ranges will work with them too.

@P4sca1 P4sca1 changed the base branch from main to next March 30, 2023 19:12
@P4sca1 P4sca1 changed the title Bug/handle 7 in weekday range fix: correctly parse weekday ranges that include sunday Mar 30, 2023
@P4sca1 P4sca1 merged commit 2fb78b5 into P4sca1:next Mar 30, 2023
P4sca1 pushed a commit that referenced this pull request Mar 30, 2023
# [4.0.0-next.3](v4.0.0-next.2...v4.0.0-next.3) (2023-03-30)

### Bug Fixes

* correctly parse weekday ranges that include sunday ([#283](#283)) ([2fb78b5](2fb78b5))
@P4sca1
Copy link
Owner

P4sca1 commented Mar 30, 2023

🎉 This PR is included in version 4.0.0-next.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

P4sca1 pushed a commit that referenced this pull request Mar 30, 2023
# [4.0.0](v3.0.6...v4.0.0) (2023-03-30)

### Bug Fixes

* correctly parse weekday ranges that include sunday ([#283](#283)) ([2fb78b5](2fb78b5))
* specify types for legacy support ([7288c13](7288c13))

### chore

* drop support for node14 ([d47ac6a](d47ac6a))

### Features

* esm only build ([58cbdd7](58cbdd7))

### BREAKING CHANGES

* If your environment does not support conditional exports, you have to import the schedulers from cron-schedule/dist/schedulers instead of cron-schedule/schedulers.

Signed-off-by: Pascal Sthamer <[email protected]>
* Support for node14 is dropped,
due to its EOL in the next month and missing ESM module support

Signed-off-by: Pascal Sthamer <[email protected]>
* The package is now ESM only. A CommonJS or IIFE build is no longer provided.
* The minimum required version of Node.js is now 14
* utils are no longer exposed
* schedulers are no longer exposed in the default entry point.
Import them from `cron-schedule/schedulers/interval-based.js` and
`cron-schedule/schedulers/timer-based.js` instead.

Signed-off-by: Pascal Sthamer <[email protected]>
@P4sca1
Copy link
Owner

P4sca1 commented Mar 30, 2023

🎉 This PR is included in version 4.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Weekday range to sunday edge case
2 participants