Skip to content

build: bazel support for importing to envoy repository.#73

Merged
htuch merged 2 commits intostagingfrom
api-import
Jun 2, 2017
Merged

build: bazel support for importing to envoy repository.#73
htuch merged 2 commits intostagingfrom
api-import

Conversation

@htuch
Copy link
Copy Markdown
Member

@htuch htuch commented Jun 1, 2017

No description provided.

@htuch
Copy link
Copy Markdown
Member Author

htuch commented Jun 1, 2017

@junr03 for review.

api/BUILD Outdated
@@ -1,5 +1,17 @@
load("//bazel:api_build_system.bzl", "api_proto_library")

cc_library(
Copy link
Copy Markdown
Member

@junr03 junr03 Jun 2, 2017

Choose a reason for hiding this comment

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

Do we want to bind all of these together as one cc_library, or export them individually. I ask because looking at the related envoy change envoyproxy/envoy#1039, when you are depending in eds on api_cc, you bring unrelated proto libraries in. I see the benefit of having the single bind from envoy_cc_api to api_cc, but if bazel is meant to make dependencies explicit, I would think that exporting these individually might make more sense. Thoughts?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I've switched to avoiding this. It was a convenience to avoid having to do bind for each of the libraries in the main Envoy repo, but it doesn't matter, we have to do something either side so it's fine to keep it fine grained, as you point out it's the Bazel way.

@htuch htuch merged commit 7506a93 into staging Jun 2, 2017
@htuch htuch deleted the api-import branch June 2, 2017 22:07
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