-
Notifications
You must be signed in to change notification settings - Fork 378
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
Manually vendor the required .proto files from googleapis #2765
Manually vendor the required .proto files from googleapis #2765
Conversation
This resolves some weird references to GOPATH, that while it worked, was weird (google#2763). It also simplifies new user onboarding (one less instruction to follow), makes CI more lightweight (one less big repo to clone), and avoids mismatched versions of these protos from causing havoc (because they were less hermetic before).
What was the sequence of actions you did to create this? Was it a copy-paste, or some "submodule" magic? It would be nice to expand the README a bit, to know how to update this dependency in the future. |
Yeah that's a good question. I've documented it in the README, and confirmed that the distilled steps I've put in there replace the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM in principle, see some more nits.
Still LGTM. Maybe worth mentioning in CHANGELOG? |
Added section to changelog. |
This resolves some weird references to GOPATH, that while it worked, was weird (#2763). It also simplifies new user onboarding (one less instruction to follow), makes CI more lightweight (one less big repo to clone), and avoids mismatched versions of these protos from causing havoc (because they were less hermetic before).