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!: UNIX standard alignments #667

Merged
merged 8 commits into from
Jul 20, 2023

Conversation

sheerlox
Copy link
Collaborator

@sheerlox sheerlox commented May 21, 2023

Closes #666.
Closes #475.

TODO

  • update month indexing from 0-11 (Jan-Dec) to 1-12 (Jan-Dec)
  • update day-of-week indexing from 0-6 (Sun-Sat) to 0-7 (Sun-Sun)
  • update "Cron Ranges" section of README
  • add a migration section to README

@sheerlox sheerlox force-pushed the feat-666/unix-standard-alignments branch from 46c2ddf to f64a47a Compare May 21, 2023 19:05
support day-of-week indexing 0-7 (Sun-Sun)
this only affects cron string parsing, not the way the library works under the hood
lib/time.js Outdated
@@ -671,8 +671,10 @@ function CronTime(luxon) {

_hasAll: function (type) {
const constraints = CONSTRAINTS[TIME_UNITS.indexOf(type)];
const low = constraints[0];
const high = type === 'dayOfWeek' ? constraints[1] - 1 : constraints[1];
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It bothers me that 'dayOfWeek' is the only time unit being referenced as a plain string in the code, which was until now perfectly generic. But since we support it as an upper bound for cron string parsing, it still needs its constraints to be different than before.

Copy link
Collaborator

Choose a reason for hiding this comment

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

if you want you can do something like

const dayOfWeekType = TIME_UNITS[5];
const high = type === dayOfWeekType ? constraints[1] - 1 : constraints[1];

to keep it generic

Copy link
Collaborator Author

@sheerlox sheerlox May 24, 2023

Choose a reason for hiding this comment

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

@intcreator I expanded on that idea and found something that should be better, please let me know what you think: 09e5d7c

@sheerlox
Copy link
Collaborator Author

Hey @intcreator 👋

So the changes for the month indexing were pretty straightforward as it was a simple shift.

The more complicated part was for day-of-week since I needed to add a second Sunday value. I thought about how that would impact the various functions for a while, then decided to go for a simple solution: to support 7 as a valid value when creating a cron, but then change it to a 0 so we don't need to modify the whole code to adapt for that change.

Please let me know your thoughts on this 😉

@intcreator
Copy link
Collaborator

intcreator commented May 22, 2023

looking good so far, thanks for your work on this! we'll probably want to put this and potentially other breaking changes on a v3 branch that people can test on/we can release as a preview before the official v3 release

@sheerlox sheerlox marked this pull request as ready for review May 24, 2023 21:06
@sheerlox sheerlox force-pushed the feat-666/unix-standard-alignments branch from 0cd7f87 to 7dfd34b Compare May 25, 2023 08:36
@sheerlox
Copy link
Collaborator Author

sheerlox commented May 25, 2023

rebased against master to keep the commit history linear

@sheerlox
Copy link
Collaborator Author

We might want to notify the folks at NestJS of this change once we release v3 (after the Typescript migration).

See:

@sheerlox sheerlox changed the title feat: UNIX standard alignments feat!: UNIX standard alignments Jun 26, 2023
@sheerlox sheerlox changed the base branch from master to beta July 20, 2023 17:21
@sheerlox sheerlox deleted the branch kelektiv:beta July 20, 2023 17:32
@sheerlox sheerlox closed this Jul 20, 2023
@sheerlox sheerlox reopened this Jul 20, 2023
@sheerlox sheerlox force-pushed the feat-666/unix-standard-alignments branch from 7dfd34b to 614a3de Compare July 20, 2023 18:55
@sheerlox sheerlox merged commit 96ef954 into kelektiv:beta Jul 20, 2023
10 checks passed
ncb000gt pushed a commit that referenced this pull request Jul 20, 2023
## [3.0.0-beta.1](v2.3.1...v3.0.0-beta.1) (2023-07-20)

### ⚠ Breaking changes

* UNIX standard alignments (#667)

### ✨ Features

* UNIX standard alignments ([#667](#667)) ([96ef954](96ef954))

### ⚙️ Continuous Integrations

* add support for beta & maintenance releases ([#677](#677)) ([c6fc842](c6fc842))
* setup conventional commits & release automation ([#673](#673)) ([c6f39ff](c6f39ff))

### ♻️ Chores

* update default branch name ([#678](#678)) ([7471e95](7471e95))
ncb000gt pushed a commit that referenced this pull request Jul 20, 2023
## [3.0.0-beta.1](v2.3.1...v3.0.0-beta.1) (2023-07-20)

### ⚠ Breaking changes

* UNIX standard alignments (#667)

### ✨ Features

* UNIX standard alignments ([#667](#667)) ([96ef954](96ef954))

### ⚙️ Continuous Integrations

* add support for beta & maintenance releases ([#677](#677)) ([c6fc842](c6fc842))
* setup conventional commits & release automation ([#673](#673)) ([c6f39ff](c6f39ff))

### ♻️ Chores

* update default branch name ([#678](#678)) ([7471e95](7471e95))
ncb000gt pushed a commit that referenced this pull request Jul 20, 2023
## [3.0.0-beta.1](v2.3.1...v3.0.0-beta.1) (2023-07-20)

### ⚠ Breaking changes

* UNIX standard alignments (#667)

### ✨ Features

* UNIX standard alignments ([#667](#667)) ([96ef954](96ef954))

### ⚙️ Continuous Integrations

* add support for beta & maintenance releases ([#677](#677)) ([c6fc842](c6fc842))
* setup conventional commits & release automation ([#673](#673)) ([c6f39ff](c6f39ff))

### ♻️ Chores

* update default branch name ([#678](#678)) ([7471e95](7471e95))
ncb000gt pushed a commit that referenced this pull request Jul 23, 2023
## [3.0.0-beta.1](v2.3.1...v3.0.0-beta.1) (2023-07-23)

### ⚠ Breaking changes

* UNIX standard alignments (#667)

### ✨ Features

* UNIX standard alignments ([#667](#667)) ([96ef954](96ef954))

### ⚙️ Continuous Integrations

* add support for beta & maintenance releases ([#677](#677)) ([c6fc842](c6fc842))
* setup conventional commits & release automation ([#673](#673)) ([c6f39ff](c6f39ff))

### ♻️ Chores

* update default branch name ([#678](#678)) ([7471e95](7471e95))
@ncb000gt
Copy link
Member

🎉 This PR is included in version 3.0.0-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

sheerlox added a commit that referenced this pull request Sep 27, 2023
sheerlox added a commit that referenced this pull request Sep 27, 2023
sheerlox added a commit to sheerlox/node-cron that referenced this pull request Sep 27, 2023
sheerlox added a commit to sheerlox/node-cron that referenced this pull request Sep 27, 2023
sheerlox added a commit to sheerlox/node-cron that referenced this pull request Sep 29, 2023
sheerlox added a commit to sheerlox/node-cron that referenced this pull request Sep 29, 2023
sheerlox added a commit to sheerlox/node-cron that referenced this pull request Sep 29, 2023
sheerlox added a commit to sheerlox/node-cron that referenced this pull request Sep 29, 2023
@sheerlox sheerlox mentioned this pull request Sep 29, 2023
sheerlox added a commit that referenced this pull request Sep 30, 2023
ncb000gt pushed a commit that referenced this pull request Sep 30, 2023
## [3.0.0](v2.4.4...v3.0.0) (2023-09-30)

### ⚠ Breaking changes

* `utcOffset` parameter no longer accepts a string
* `utcOffset` values between -60 and 60 are no longer
treated as hours
* providing both `timeZone` and `utcOffset` parameters
now throws an error
* removed `cron.job()` method in favor of `new CronJob(...args)` /
`CronJob.from(argsObject)`
* removed `cron.time()` method in favor of `new CronTime()`
* `CronJob`: constructor no longer accepts an object as its first and
only params. Use `CronJob.from(argsObject)` instead.
* `CronJob`: callbacks are now called in the order they were registered
* return empty array from nextDates when called without argument (#519)
* UNIX standard alignments (#667)

### ✨ Features

* expose useful types ([737b344](737b344))
* rework utcOffset parameter ([#699](#699)) ([671e933](671e933))
* UNIX standard alignments ([#667](#667)) ([ff615f1](ff615f1))

### 🐛 Bug Fixes

* return empty array from nextDates when called without argument ([#519](#519)) ([c2891ba](c2891ba))

### 📦 Code Refactoring

* migrate to TypeScript ([#694](#694)) ([2d77894](2d77894))

### 📚 Documentation

* **readme:** remove outdated informations ([#695](#695)) ([b5ceaf1](b5ceaf1))

### 🚨 Tests

* update new test for cron standard alignments ([4a406c1](4a406c1))

### ♻️ Chores

* improve GitHub community standards ([#698](#698)) ([6bdef77](6bdef77))
* update contributors list ([dab3d69](dab3d69))

### 💎 Styles

* fix linting issues ([47e665f](47e665f))
@sheerlox sheerlox added released type:feature New feature or feature improvement & requests and removed released on @beta labels Sep 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released type:feature New feature or feature improvement & requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

align with the UNIX cron format node-cron's month does not follow normal cron spec
3 participants