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

Import GRPC and a simple core.proto file #1651

Closed
wants to merge 1 commit into from
Closed

Import GRPC and a simple core.proto file #1651

wants to merge 1 commit into from

Conversation

riking
Copy link
Contributor

@riking riking commented Mar 24, 2016

This splits out the godeps changes from #1647, and adds make grpc to the Makefile, which will be used to rebuild all protobuf files.

@letsencryptbot
Copy link

rpc/pb/core.pb.go:31:6: exported type KeyAuthorization should have comment or be unexported rpc/pb/core.pb.go:36:1: exported method KeyAuthorization.Reset should have comment or be unexported rpc/pb/core.pb.go:38:1: exported method KeyAuthorization.ProtoMessage should have comment or be unexported rpc/pb/core.pb.go:39:1: exported method KeyAuthorization.Descriptor should have comment or be unexported rpc/pb/core.pb.go:41:6: exported type Domain should have comment or be unexported rpc/pb/core.pb.go:45:1: exported method Domain.Reset should have comment or be unexported rpc/pb/core.pb.go:47:1: exported method Domain.ProtoMessage should have comment or be unexported rpc/pb/core.pb.go:48:1: exported method Domain.Descriptor should have comment or be unexported rpc/pb/core.pb.go:50:6: exported type Valid should have comment or be unexported rpc/pb/core.pb.go:54:1: exported method Valid.Reset should have comment or be unexported rpc/pb/core.pb.go:56:1: exported method Valid.ProtoMessage should have comment or be unexported rpc/pb/core.pb.go:57:1: exported method Valid.Descriptor should have comment or be unexported

@letsencryptbot
Copy link

rpc/pb/core.pb.go:31:6: exported type KeyAuthorization should have comment or be unexported rpc/pb/core.pb.go:36:1: exported method KeyAuthorization.Reset should have comment or be unexported rpc/pb/core.pb.go:38:1: exported method KeyAuthorization.ProtoMessage should have comment or be unexported rpc/pb/core.pb.go:39:1: exported method KeyAuthorization.Descriptor should have comment or be unexported rpc/pb/core.pb.go:41:6: exported type Domain should have comment or be unexported rpc/pb/core.pb.go:45:1: exported method Domain.Reset should have comment or be unexported rpc/pb/core.pb.go:47:1: exported method Domain.ProtoMessage should have comment or be unexported rpc/pb/core.pb.go:48:1: exported method Domain.Descriptor should have comment or be unexported rpc/pb/core.pb.go:50:6: exported type Valid should have comment or be unexported rpc/pb/core.pb.go:54:1: exported method Valid.Reset should have comment or be unexported rpc/pb/core.pb.go:56:1: exported method Valid.ProtoMessage should have comment or be unexported rpc/pb/core.pb.go:57:1: exported method Valid.Descriptor should have comment or be unexported

@rolandshoemaker
Copy link
Contributor

(You also need to disable the go lint testing for this to pass, but first...)

I'm not sure this is the best approach. I think we should probably wait to vendor everything once we actually have a PR that's ready to land.

Also I think we should discuss the best way to organize protobuf files out of band, lumping everything into a single giant file is going to lead to having a mess of the same size as the current RPC layer (albiet somewhat easier to understand).

Given we already split each service into its own subpackage why not have a single protobuf file for each service? This should reduce confusion, and more importantly the compulsion to copy and paste everything. Since protobuf definitions allow imports we can also bypass repeated definitions of core types by having a protobuf companion to core/objects.go which has the canonical definitions.

@rolandshoemaker
Copy link
Contributor

I'd also recommend that instead of using sed you just use a godep save -r ./... since it'll catch any other imports we are unaware of protobuf pulling in itself in the future.

@letsencryptbot
Copy link

rpc/pb/core.pb.go:36:6: exported type KeyAuthorization should have comment or be unexported rpc/pb/core.pb.go:41:1: exported method KeyAuthorization.Reset should have comment or be unexported rpc/pb/core.pb.go:43:1: exported method KeyAuthorization.ProtoMessage should have comment or be unexported rpc/pb/core.pb.go:44:1: exported method KeyAuthorization.Descriptor should have comment or be unexported rpc/pb/core.pb.go:46:6: exported type Domain should have comment or be unexported rpc/pb/core.pb.go:50:1: exported method Domain.Reset should have comment or be unexported rpc/pb/core.pb.go:52:1: exported method Domain.ProtoMessage should have comment or be unexported rpc/pb/core.pb.go:53:1: exported method Domain.Descriptor should have comment or be unexported rpc/pb/core.pb.go:55:6: exported type Valid should have comment or be unexported rpc/pb/core.pb.go:59:1: exported method Valid.Reset should have comment or be unexported rpc/pb/core.pb.go:61:1: exported method Valid.ProtoMessage should have comment or be unexported rpc/pb/core.pb.go:62:1: exported method Valid.Descriptor should have comment or be unexported rpc/pb/core.pb.go:80:6: exported type DummyServiceClient should have comment or be unexported rpc/pb/core.pb.go:88:1: exported function NewDummyServiceClient should have comment or be unexported rpc/pb/core.pb.go:103:6: exported type DummyServiceServer should have comment or be unexported rpc/pb/core.pb.go:107:1: exported function RegisterDummyServiceServer should have comment or be unexported rpc/pb/core.pb.go:111:6: don't use underscores in Go names; func _DummyService_Dummy_Handler should be _DummyServiceDummyHandler rpc/pb/core.pb.go:123:5: don't use underscores in Go names; var _DummyService_serviceDesc should be _DummyServiceServiceDesc

@riking
Copy link
Contributor Author

riking commented Mar 24, 2016

Also I think we should discuss the best way to organize protobuf files out of band, lumping everything into a single giant file is going to lead to having a mess of the same size as the current RPC layer (albiet somewhat easier to understand).

why not have a single protobuf file for each service?

Yeah, that was the plan. core.proto plus service_name.proto, but all files inside rpc/pb.

I'd also recommend that instead of using sed you just use a godep save -r ./... since it'll catch any other imports we are unaware of protobuf actually pulling in itself in the future.

Er, vendored dependencies don't update by themselves.

@riking riking closed this Mar 25, 2016
@jsha jsha deleted the add-grpc branch July 13, 2020 23:56
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.

3 participants