search

adhocore / gronx

Lightweight, fast and dependency-free Cron expression parser (due checker, next/prev due date finder), task runner, job scheduler and/or daemon for Golang (tested on v1.13+) and standalone usage. If you are bold, use it to replace crontab entirely.
https://github.com/adhocore/gronx
MIT License
422 stars 25 forks source link

Fix bump segments iteration order and day of month/week intersection/union handling #35

Closed codenem closed 8 months ago

codenem commented 9 months ago

Fixes #33

Issue is that sometimes changing a part of the ref date should also change other parts. Example: If we bump year while computing next tick, we must then put ref to 1st January of next year, at midnight. To implement this logic the iteration over segments have been reversed to start with segments corresponding to bigger time periods, and the bump function has been updated to handle this new logic of dependent segment changes.

Fixes #26

Issue was that when using steps for months (e.g. 3/*) the fact that months start at 1 (January) and not 0 must be taken into account.

About day of week and day of month

So according to crontab guru (see tip 1 in https://crontab.guru/tips.html) and also general crontab standard, day of week and day of month are intertwined. So if any starts with * the computed day must match the intersection of the two. If not, it must match the union.

adhocore commented 9 months ago

wow, thanks for this 👍. will take a look closely later when im in a PC

codecov-commenter commented 9 months ago

Codecov Report

Attention: Patch coverage is 96.93878% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 93.88%. Comparing base (d3b6afc) to head (d971949). Report is 2 commits behind head on main.

Files Patch % Lines
next.go 95.23% 1 Missing and 2 partials :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #35 +/- ## ========================================== - Coverage 94.78% 93.88% -0.90% ========================================== Files 9 9 Lines 594 671 +77 ========================================== + Hits 563 630 +67 - Misses 16 22 +6 - Partials 15 19 +4 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

adhocore commented 9 months ago

i have yet to review but wanted to let the actions run thru. it's great that tests/builds passed, only a little comment from coverage bot codecov.

codenem commented 9 months ago

i have yet to review but wanted to let the actions run thru. it's great that tests/builds passed, only a little comment from coverage bot codecov.

Understood thanks! I will add the missing coverage.

codenem commented 8 months ago

@adhocore added new tests for coverage

adhocore commented 8 months ago

thank you for clarification. seems now we are near merging this PR. while it is more of personal preference, is there a way to make some var name shorter instead like monthDaySegIsIntersecting