search

kiorky / croniter

MIT License
382 stars 37 forks source link

support year field #80

Closed JabberWocky-22 closed 1 month ago

JabberWocky-22 commented 2 months ago

support year at the seventh field.

kiorky commented 2 months ago

i'm willing to accept this, but i have to review it carefully.

Please make sure to add extensive tests, cron is devil in case of matrix cases ;-)

JabberWocky-22 commented 2 months ago

I have rebased to master, fixed the test and added more. Please take some time to review :) @kiorky

Besides: I also fixed a bug which https://github.com/kiorky/croniter/pull/42 doesn't cover(range) Ensure match_range return False when not previous value avaliable Sorry for mix them up.

kiorky commented 2 months ago

I have rebased to master, fixed the test and added more. Please take some time to review :) @kiorky

Besides: I also fixed a bug which #42 doesn't cover(range) Ensure match_range return False when not previous value avaliable Sorry for mix them up.

yes i saw, this worries me, can you make a separate PR for this, i ll merge it way faster that this one that should be scopped as it is already invasive.

kiorky commented 2 months ago

but how did you rebase, as there are 3 commits, can you just make a PR with the match_range problem, then rebase (as git rebase -i) on top of it ?

JabberWocky-22 commented 2 months ago

I have made 2 separate bug fix PR.

I just rebase this PR to master and didn't squash the commits. Some projects dislike squash and force-push. I will squash it after it's ok.

kiorky commented 2 months ago

Damn, i m so sorry, seems the review i already did yesterday wasnt published. So some (yesterday) comments may be out of date.

What i propose to you is to make the other PR #82 merged, then you rebase this PR, then i ll ensure the review is up to date ...

kiorky commented 2 months ago

So please now rebase this one to see the final diff that should have a lift :)

kiorky commented 2 months ago

(you should obtain something similar to https://github.com/kiorky/croniter/compare/master...y )

kiorky commented 1 month ago

There is a bug with the comment posts, give me a minute to sort it out ...

kiorky commented 1 month ago

There is a bug with the comment posts, give me a minute to sort it out ...

you can proceed, sorry for the noise :)

kiorky commented 1 month ago

So, so far so good, merging, and a lot of thanks for applying those review modifications, along to the 2 other PRs !

kiorky commented 1 month ago

released as 3.0.0 https://pypi.org/project/croniter/3.0.0/