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 go_package declarations #1593

Merged
merged 2 commits into from
Oct 23, 2017
Merged

Update go_package declarations #1593

merged 2 commits into from
Oct 23, 2017

Conversation

tomwilkie
Copy link
Contributor

The expected contents of go_package has changed, see golang/protobuf#139

This is needed to build stuff with bazel - see bazelbuild/rules_go#836

@@ -15,7 +15,7 @@
syntax = "proto3";

package grpc.lb.v1;
option go_package = "messages";
option go_package = "google.golang.org/grpc/grpclb/grpc_lb_v1/messages";
Copy link
Contributor

Choose a reason for hiding this comment

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

Changing this option also changes the output file path. (Run make proto to see the result).

It seems our current proto structure (or just the way we run protoc) doesn't work well with this new go_package. We need to fix that.

Copy link
Contributor

@menghanl menghanl Oct 19, 2017

Choose a reason for hiding this comment

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

The fix for this is to change the --go_out parameter for go generate in grpclb_test.go

diff --git a/grpclb/grpclb_test.go b/grpclb/grpclb_test.go
index 46c1fe5..cc8d75c 100644
--- a/grpclb/grpclb_test.go
+++ b/grpclb/grpclb_test.go
@@ -16,8 +16,8 @@
  *
  */
 
-//go:generate protoc --go_out=plugins=:. grpc_lb_v1/messages/messages.proto
-//go:generate protoc --go_out=Mgrpc_lb_v1/messages/messages.proto=google.golang.org/grpc/grpclb/grpc_lb_v1/messages,plugins=grpc:. grpc_lb_v1/service/service.proto
+//go:generate protoc --go_out=plugins=:$GOPATH grpc_lb_v1/messages/messages.proto
+//go:generate protoc --go_out=plugins=grpc:$GOPATH grpc_lb_v1/service/service.proto
 
 // Package grpclb_test is currently used only for grpclb testing.
 package grpclb_test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice, thanks!

Copy link
Contributor

@menghanl menghanl left a comment

Choose a reason for hiding this comment

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

LGTM

@menghanl menghanl merged commit 0c2d9db into grpc:master Oct 23, 2017
@menghanl menghanl added this to the 1.8 Release milestone Nov 7, 2017
@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants