-
Notifications
You must be signed in to change notification settings - Fork 191
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
regen protos with protoc 1.3.1 #164
Conversation
Do we know if we're good to move forward with this PR? |
@tbpg Since we've just released, I'd say it's 👍 to go ahead with this and see what breaks. If something is hugely wrong with this we can roll back before v0.40.0. :) |
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 assuming tests are green.
This allows removal of x/net/sync and x/net/context, which is a welcome change. BREAKING_CHANGE_ACCEPTABLE=protoc churn
Relevant context: googleapis/go-genproto#164 Also removes unnecessary (and troublesome) go1.12 line in v2/.
Relevant context: googleapis/go-genproto#164 Also removes unnecessary (and troublesome) go1.12 line in v2/.
Relevant context: googleapis/go-genproto#164 Also removes unnecessary (and troublesome) go1.12 line in v2/.
You broke our build. Also there is no tag of the 1.2.* version so we cannot even pin to that. |
Please file a bug including the failure and all versions of relevant code you're using ( |
Relevant context: googleapis/go-genproto#164 Change-Id: I238304d9150454b8e09dca9b02f844eb95f77754 Reviewed-on: https://code-review.googlesource.com/c/gocloud/+/41170 Reviewed-by: kokoro <noreply+kokoro@google.com> Reviewed-by: Tyler Bui-Palsulich <tbp@google.com>
Not at all clear to me yet whether this change is safe or not.