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

chore: Run update-bins and codegen. #388

Merged
merged 1 commit into from
Nov 15, 2021
Merged

Conversation

stephenh
Copy link
Owner

Not entirely sure what happened, but unfortunately our bin files are dependent on the locally installed version of protoc, and so at times we get slightly different versions of them checked in.

Eventually checking in the .bin files is a hack that should just go away, and we should like a docker-hosted version of protoc so that all builds, local or CI, use the same protoc for the integration test files.

But, for now, just update them to my local protoc (3.12.4) + codegen against them.

The local change is a comment change noticed in #387.

@webmaster128
Copy link
Collaborator

👍

Do you know where protoc comes from in the CI? Is that magically installed on ubuntu-latest?

@stephenh
Copy link
Owner Author

@webmaster128 honestly I don't think protoc is currently available in CI / on those images, i.e. part of the "cute" "just check in the bins" approach was besides collaborators not needing a protoc setup to do PRs (...granted, ones that don't involve new .proto file test cases, which I guess is probably a small subset of PRs in retrospect), it meant I also didn't have to figure out how to get protoc into CI either. :-)

Granted, it's probably just doing something like apt install protoc, but again disclaimer this is just what seemed easiest at the time.

A few months ago I briefly looked into moving things over to "something something docker", per the PR description, so that local PRs and CI builds would for sure use the same version of protoc, but didn't end up following through on that musing...

@stephenh stephenh merged commit b2499a7 into main Nov 15, 2021
@stephenh stephenh deleted the update-bins-and-codegen branch November 15, 2021 18:00
@webmaster128
Copy link
Collaborator

Ahh, then it makes sense. Thanks for the explanation.

@stephenh
Copy link
Owner Author

🎉 This PR is included in version 1.86.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants