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

Fix river.JobStateAvailable reference in cmd/river/ #315

Merged
merged 2 commits into from
Apr 23, 2024

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Apr 22, 2024

@@ -1465,7 +1465,7 @@ type JobListResult struct {
// provided context is used for the underlying Postgres query and can be used to
// cancel the operation or apply a timeout.
//
// params := river.NewJobListParams().WithLimit(10).State(river.JobStateCompleted)
// params := river.NewJobListParams().WithLimit(10).State(rivertype.JobStateCompleted)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found these via git grep -n river.JobState

Copy link
Contributor

@bgentry bgentry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for submitting a fix so quickly! 🙏🏻

@dhermes
Copy link
Contributor Author

dhermes commented Apr 22, 2024

@bgentry You've got to approve CI to run here

(Thanks for the LGTM BTW 😆)

@dhermes
Copy link
Contributor Author

dhermes commented Apr 22, 2024

Oh shoot, I can't merge it either, sorry for the ping @bgentry!

Screenshot 2024-04-22 at 1 59 58 PM

@brandur
Copy link
Contributor

brandur commented Apr 23, 2024

Thanks @dhermes.

Out of curiosity, you said you actually ran into an error because of this? I would not have expected this to happen because unlike other internal Go modules, ./cmd/river is affixed to a particular version of other River packages (still 0.1.0 currently). That affixed version didn't change, so the removal of the river.JobState* aliases from the most recent version was safe and should have had no affect on it. I tried go install ... and it still seems to work happily.

Do you have something exotic going on in your build, or am I missing something?

@brandur brandur merged commit 7c8b5ee into riverqueue:master Apr 23, 2024
10 checks passed
brandur added a commit that referenced this pull request Apr 23, 2024
A small release containing the change in #315. I don't _think_ anything
critical was broken, but not completely certain, and a release is an
easy thing to do, so just in case.

Also update `./cmd/river`'s references to `v0.4.0` which shouldn't
affect anything, but a minor nicety to stay current.
@brandur brandur mentioned this pull request Apr 23, 2024
brandur added a commit that referenced this pull request Apr 23, 2024
A small release containing the change in #315. I don't _think_ anything
critical was broken, but not completely certain, and a release is an
easy thing to do, so just in case.

Also update `./cmd/river`'s references to `v0.4.0` which shouldn't
affect anything, but a minor nicety to stay current.
brandur added a commit that referenced this pull request Apr 23, 2024
A small release containing the change in #315. I don't _think_ anything
critical was broken, but not completely certain, and a release is an
easy thing to do, so just in case.

Also update `./cmd/river`'s references to `v0.4.0` which shouldn't
affect anything, but a minor nicety to stay current.
@dhermes dhermes deleted the patch-1 branch April 23, 2024 02:17
@dhermes
Copy link
Contributor Author

dhermes commented Apr 23, 2024

@brandur We have cmd/river/ in our go.mod + vendor/, and there isn't a mix-and-match of dependencies in a go.mod (so if riverqueue/river gets updated, it's global).

We vendor in cmd/river/ so that in CI we don't end up having to do separate out-of-band Go build steps for our "check if river migrations are valid" (separate from building our source tree).

I suppose the build cost could be paid upfront and placed in a GitHub Action (e.g. via something like https://full-stack.blend.com/how-we-write-github-actions-in-go.html), just haven't realized there was a need until (maybe) right now.

@brandur
Copy link
Contributor

brandur commented Apr 23, 2024

@dhermes Ah gotcha! Okay that makes sense. Thanks for explaining.

brandur added a commit that referenced this pull request Apr 24, 2024
A very small one: I forgot to add changelog attribution for #315. This
seems like a nice convention for recognizing third party contributors,
so here, put some in.
brandur added a commit that referenced this pull request Apr 24, 2024
A very small one: I forgot to add changelog attribution for #315. This
seems like a nice convention for recognizing third party contributors,
so here, put some in.
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.

None yet

3 participants