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

Update opencensus-proto dependency to v0.1.0 release. #5282

Merged
merged 2 commits into from
Dec 19, 2018

Conversation

g-easy
Copy link
Contributor

@g-easy g-easy commented Dec 13, 2018

Signed-off-by: Emil Mikulic g-easy@users.noreply.github.com

Part of #2456

@g-easy
Copy link
Contributor Author

g-easy commented Dec 13, 2018

census-instrumentation/opencensus-proto#61 changed

option go_package = "traceproto";

to the fully qualified

option go_package = "github.com/census-instrumentation/opencensus-proto/gen-go/trace/v1";

and this broke protoc-gen-validate:

ERROR: output 'external/io_opencensus_trace/trace.pb.validate.h' was not created

I don't know enough about go_package or protoc-gen-validate; could someone please help me: what's the way forward here?

cc @rakyll

@zuercher
Copy link
Member

zuercher commented Dec 13, 2018

Looks like you're hitting bufbuild/protoc-gen-validate#41 (per @htuch's research)

@htuch
Copy link
Member

htuch commented Dec 14, 2018

@akonradi @rodaine any thoughts on how best to tackle bufbuild/protoc-gen-validate#41?

@rodaine
Copy link
Member

rodaine commented Dec 17, 2018

Similar to the override presented here for the Java path, something similar should be done for C++ (otherwise, it'll follow the protoc-gen-go rules): https://github.com/lyft/protoc-gen-validate/blob/a346734/templates/pkg.go#L34

@htuch
Copy link
Member

htuch commented Dec 18, 2018

@rodaine I'll give this a go this arvo.

Signed-off-by: Emil Mikulic <g-easy@users.noreply.github.com>
Signed-off-by: Emil Mikulic <g-easy@users.noreply.github.com>
Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

Looks like this passed the API tests, so good to go.

@g-easy
Copy link
Contributor Author

g-easy commented Dec 19, 2018

Thanks for the PGV fix!

I'm not authorized to merge this pull request.

@htuch htuch merged commit 4b47597 into envoyproxy:master Dec 19, 2018
@g-easy g-easy deleted the proto branch December 20, 2018 01:36
fredlas pushed a commit to fredlas/envoy that referenced this pull request Mar 5, 2019
Part of envoyproxy#2456

Signed-off-by: Emil Mikulic <g-easy@users.noreply.github.com>
Signed-off-by: Fred Douglas <fredlas@google.com>
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

4 participants