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

remove the now > start check in RFC5545 #1176

Merged
merged 4 commits into from Sep 1, 2016

Conversation

Projects
None yet
2 participants
@ssalinas
Member

ssalinas commented Jul 28, 2016

Need to loop from the real start each time to get the scheduling correct.

@grepsr can test and confirm that this works the way you would expect?

/cc @tpetr

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Jul 29, 2016

@ssalinas Thanks I will test this!

Regarding MAX_ITERATIONS = 10000, I think 10k is a small number.

If someone wants to do an hourly job, it will stop working after 1.1 years

Since the loop will return midway, once a matching interval is found - it will hardly ever loop a lot?
Perhaps we should just increase the number to MAX_ITERATIONS = 1000000?
This way it will work properly for 114 years lol - someone else can do a PR then to fix it ;-)

Hate to have so many loops, but if you think of it, the cron library probably loops internally like this too - so it seems ok?

ghost commented Jul 29, 2016

@ssalinas Thanks I will test this!

Regarding MAX_ITERATIONS = 10000, I think 10k is a small number.

If someone wants to do an hourly job, it will stop working after 1.1 years

Since the loop will return midway, once a matching interval is found - it will hardly ever loop a lot?
Perhaps we should just increase the number to MAX_ITERATIONS = 1000000?
This way it will work properly for 114 years lol - someone else can do a PR then to fix it ;-)

Hate to have so many loops, but if you think of it, the cron library probably loops internally like this too - so it seems ok?

ssalinas added some commits Jul 29, 2016

@ssalinas

This comment has been minimized.

Show comment
Hide comment
@ssalinas

ssalinas Jul 29, 2016

Member

@grepsr bumped the max iterations as well.

Member

ssalinas commented Jul 29, 2016

@grepsr bumped the max iterations as well.

@ssalinas ssalinas added the hs_staging label Jul 29, 2016

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Jul 29, 2016

Awesome, thanks!

ghost commented Jul 29, 2016

Awesome, thanks!

@ssalinas ssalinas modified the milestone: 0.10.0 Jul 29, 2016

@ssalinas ssalinas modified the milestones: 0.10.0, 0.11.0 Aug 19, 2016

@ssalinas ssalinas merged commit 4f4462a into master Sep 1, 2016

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@ssalinas ssalinas deleted the rfc_fix branch Sep 1, 2016

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