-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
gazelle: use default proto generation except for testgrid/config #6191
Conversation
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
/hold
/area bazel |
This collides with #6149 and I'm not sure how to address that. /shrug |
Actually, running |
e30ec15
to
8c0d9bb
Compare
|
I think I figured it out - we needed one more call to |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: BenTheElder, fejta, ixdy The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
/hold cancel |
vendor/BUILD
Outdated
@@ -43,7 +43,6 @@ filegroup( | |||
"//vendor/github.com/matttproud/golang_protobuf_extensions/pbutil:all-srcs", | |||
"//vendor/github.com/peterbourgon/diskv:all-srcs", | |||
"//vendor/github.com/prometheus/client_golang/prometheus:all-srcs", | |||
"//vendor/github.com/prometheus/client_model:all-srcs", |
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.
Shouldn't this also update the dep files?
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.
Line 187 in cab53df
name = "github.com/prometheus/client_model" |
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.
yeah, it should have.
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.
Also, this library seems to be used transitively by github.com/prometheus/client_golang
.
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.
OK, this seems to be added back in this very same PR. Sorry for the noise.
We should also fix
testgrid/config
at some point, but that's slightly more involved, as I noted in https://kubernetes.slack.com/archives/C09QZ4DQB/p1513824406000116./assign @BenTheElder @fejta