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.

Sign up for GitHub

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

Jump to bottom

Update README to mention second-precision jobs #240

Merged
merged 1 commit into from Apr 12, 2022
Merged

Update README to mention second-precision jobs #240

merged 1 commit into from Apr 12, 2022

Conversation

tekin
Copy link
Contributor

@tekin tekin commented Feb 25, 2019

Updates the README to mention this undocumented (and regularly requested!) feature.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 91.688% when pulling 2f52099 on tekin:mention-six-parameter-cronline-in-readme into 25d7bc7 on ondrejbartas:master.

@tekin
Copy link
Contributor Author

tekin commented Mar 19, 2019

@ondrejbartas coverage has dropped slightly, but the PR is only updating the README. Thoughts?

@jmettraux jmettraux mentioned this pull request Mar 21, 2019
Closed
@gogiel
Copy link

gogiel commented Aug 1, 2019 •
edited

Thank you for this. I can confirm that it works. Anything blocking the merge?

@tekin
Copy link
Contributor Author

tekin commented Aug 2, 2019

@gogiel not from my perspective.

@kaka-ruto kaka-ruto mentioned this pull request Mar 28, 2021
Closed
@beatak
Copy link

beatak commented Dec 3, 2021

This should be closed and deleted because .poll_interval was long gone by v3.4.0

https://github.com/mperham/sidekiq/blob/main/Changes.md#340

which was from 2015.

sidekiq/sidekiq@1499493

@tekin tekin force-pushed the mention-six-parameter-cronline-in-readme branch from 92d60ff to 3e417b4 Compare January 4, 2022 16:58
@tekin
Copy link
Contributor Author

tekin commented Jan 4, 2022

@beatak thanks for pointing that out. Rather than delete this pull request I've updated it to mention the new configuration mechanism. I've left the PR open as the main point of the PR (to explain how to schedule second-precision jobs) still stands.

@borisromanov
Copy link

Hi, just stumbling across this now. Any issues with merging this PR so the sidekiq-cron community can benefit?

@honzasterba
Copy link
Collaborator

needs to be rebased and can be merged

@markets
Copy link
Member

markets commented Apr 11, 2022

We also mention the average_scheduled_poll_interval options here https://github.com/ondrejbartas/sidekiq-cron#under-the-hood, but with different syntax and different defaults 🤔 should we "unify" or DRY these explanations?

@tekin tekin force-pushed the mention-six-parameter-cronline-in-readme branch from 3e417b4 to 966c223 Compare April 12, 2022 13:16
@tekin
Copy link
Contributor Author

tekin commented Apr 12, 2022

@honzasterba / @markets thanks both. I've resolved the merge conflict and adjusted the change to use the same syntax as the "under the hood" section.

@tekin
Update README to mention second-precision jobs
d91d1af 
Lack of support for this keeps getting raised as an issue on GitHub,
despite the fact it is in fact supported:

  sidekiq-cron#103
  sidekiq-cron#67
  sidekiq-cron#26
@tekin tekin force-pushed the mention-six-parameter-cronline-in-readme branch from 966c223 to d91d1af Compare April 12, 2022 13:19
@honzasterba honzasterba merged commit d1858d4 into sidekiq-cron:master Apr 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@markets markets markets approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@tekin @coveralls @gogiel @beatak @borisromanov @honzasterba @markets