Skip to content

Add missing BUILD rules for service config proto.#74

Merged
easwars merged 1 commit intogrpc:masterfrom
easwars:service_config_build
Feb 11, 2020
Merged

Add missing BUILD rules for service config proto.#74
easwars merged 1 commit intogrpc:masterfrom
easwars:service_config_build

Conversation

@easwars
Copy link
Copy Markdown
Contributor

@easwars easwars commented Feb 7, 2020

No description provided.

@ejona86
Copy link
Copy Markdown
Member

ejona86 commented Feb 7, 2020

What needs to use this?

@easwars
Copy link
Copy Markdown
Contributor Author

easwars commented Feb 10, 2020

There is no active user of this. But the fact that when I made a change to the proto, there was no way to verify that my changes would build. That's the reason.

@dfawley
Copy link
Copy Markdown
Member

dfawley commented Feb 10, 2020

What needs to use this?

no way to verify that my changes would build

So the short answer is actually "travis":

[213 / 223] 2 actions, 1 running
    @com_google_protobuf//:protobuf_java; 1s linux-sandbox
    [Prepa] Generating Descriptor Set proto_library //:service_config_proto

This means it's working, as these lines do not appear in the master build.

Copy link
Copy Markdown
Member

@markdroth markdroth left a comment

Choose a reason for hiding this comment

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

Thanks for doing this!

Copy link
Copy Markdown
Member

@ejona86 ejona86 left a comment

Choose a reason for hiding this comment

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

Dependencies need to be exposed via a helper library so that downstream users can pull in the expected versions. But this dependency is not needed by downstream users unless they use service config. Once we actually need this dependency downstream we'll need to create a helper library.

@easwars easwars merged commit e53e1f3 into grpc:master Feb 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants