Skip to content

api filter http: add build rules for go protobufs#7526

Merged
htuch merged 2 commits intoenvoyproxy:masterfrom
nareddyt:filter-add-go-proto-library
Jul 12, 2019
Merged

api filter http: add build rules for go protobufs#7526
htuch merged 2 commits intoenvoyproxy:masterfrom
nareddyt:filter-add-go-proto-library

Conversation

@nareddyt
Copy link
Copy Markdown
Contributor

Description: All http filters have build rules to generate cc protobufs, but not go protobufs. Added build rules (to a few filters) to generate go protobuf files. Emulates the rules in the health_check http filter.

Risk Level: Low

Testing: These rules were copied to google3 and tested internally. Unfortunately, I am having a bit of trouble with bazel build directly on these targets ("Package is considered deleted due to --deleted_packages"). Please let me know if there is a better way to test this change.

Docs Changes: N/A

Release Notes: N/A

All http filters have build rules to generate cc protobufs, but not go protobufs. Added build rules to generate go protobuf files. Emulates the rules in the `health_check` http filter.

Signed-off-by: Teju Nareddy <nareddyt@google.com>
Copy link
Copy Markdown
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.

Thanks, needs to pass CI though.
/wait

Signed-off-by: Teju Nareddy <nareddyt@google.com>
Copy link
Copy Markdown
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.

Thanks!

],
)

api_go_proto_library(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@kyessenov how come we don't just have this embedded in api_proto_library_internal? This seems like a bunch of boilerplate that we could potentially eliminate.

@htuch htuch merged commit 23d82b9 into envoyproxy:master Jul 12, 2019
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.

2 participants