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

Rename RunImmediately to RunOnStart #8

Merged
merged 1 commit into from
Nov 11, 2023
Merged

Conversation

brandur
Copy link
Contributor

@brandur brandur commented Nov 11, 2023

This is another bikeshed issue, but I was thinking about the naming of
RunImmediately today and I think it might be too ambiguous. It sounds
a little too much like it could mean "run immediately when scheduled".

I propose we rename it to RunOnStart to make it clear that it's
expected to run on the scheduler's start (this is also said in the
property's docblock). RunOnStart is also a little more succinct which
is always nice.

The repository is technically public now and this is technically a
breaking change, but I think it's fair for us to make a few API tweaks
in the early days before we've done anything to send the project around
(and maybe even a little after).

This property in mentioned in our public docs already [1]. If this gets
merged I'll follow up by updating it there as well.

[1] https://riverqueue.com/docs/periodic-jobs

@brandur brandur requested a review from bgentry November 11, 2023 01:41
@brandur
Copy link
Contributor Author

brandur commented Nov 11, 2023

@bgentry Thoughts on this one?

This is another bikeshed issue, but I was thinking about the naming of
`RunImmediately` today and I think it might be too ambiguous. It sounds
a little too much like it could mean "run immediately when scheduled".

I propose we rename it to `RunOnStart` to make it clear that it's
expected to run on the scheduler's start (this is also said in the
property's docblock). `RunOnStart` is also a little more succinct which
is always nice.

The repository is technically public now and this is technically a
breaking change, but I think it's fair for us to make a few API tweaks
in the early days before we've done anything to send the project around
(and maybe even a little after).

This property in mentioned in our public docs already [1]. If this gets
merged I'll follow up by updating it there as well.

[1] https://riverqueue.com/docs/periodic-jobs
@brandur brandur merged commit 78dea02 into master Nov 11, 2023
5 checks passed
@brandur brandur deleted the brandur-run-on-start branch November 11, 2023 02:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants