-
Notifications
You must be signed in to change notification settings - Fork 290
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
Remove options in Managed Mode #645
Comments
For future reference, this is roughly what we want: diff --git a/data/template/buf.go-client.gen.yaml b/data/template/buf.go-client.gen.yaml
index 68b84740..7b522a82 100644
--- a/data/template/buf.go-client.gen.yaml
+++ b/data/template/buf.go-client.gen.yaml
@@ -5,6 +5,12 @@ managed:
enabled: true
go_package_prefix:
default: github.com/bufbuild/buf/private/gen/proto/go
+ options:
+ exclude:
+ - (validate.rules)
+ imports:
+ exclude:
+ - validate/validate.proto
plugins:
- name: go-grpc
out: private/gen/proto/go
|
At a high level, the proposal is fairly easy to implement - we can modify the For example, suppose that I strip the $ buf generate proto --template data/template/buf.go.gen.yaml
protoc-gen-go: invalid FileDescriptorProto "buf/alpha/image/v1/image.proto": proto: message field "buf.alpha.image.v1.ImageFile.message_type" cannot resolve type: "google.protobuf.DescriptorProto" not found We could consider this a user error and justify that the feature is working as expected, but it's still not an ideal user experience - the plugins could now receive a malformed Removing options is also strange - we would need to clear the extension from the In order to satisfy both of these requirements, we'd need to verify that the We should consider whether or not the complexity in this feature is worth it. There's already a fair amount of learning required for users to understand Managed Mode, and this adds even more questions into the mix. I'm not so worried about the implementation in this case - I'm only concerned about the UX. |
Another thing to keep in mind here - We're beginning to see similar functionality appear in different parts of Similarly, this might be a feature that we want to implement on the AST before it's compiled into an TL;DR We'll need to think about this one a bit more to decide where it should go. |
Another case where this feature would shine is docs and api spec generators like gnostic. You wouldn't need them in the language-specific runtime. |
I found this issue while looking to file a similar request. Like in the original comment, we too would like to eliminate all This is to help reduce the code size of our clients - for some languages like Ruby, the entire descriptor is emitted into the generated code, even though Ruby can't make use of the I'd be happy for this to either be a transform on the AST, or on the |
A few thoughts here:
|
We recently had a feature request related to PHP code generation and protoc-gen-validate. In summary, PHP can't generate code for
proto2
files, so anything that dependsprotoc-gen-validate
will not compile.We want users to incorporate
protoc-gen-validate
as they please, but it's really only relevant when generating the server implementation in the user's language of choice (go
in this case).We've discussed managing
.proto
options in general (not just file-level options), so this is a good first candidate to explore that space. In this case, we would need to remove all instances of a particular set of options (those provided byvalidate.rules
) and we would need to remove theimport validate/validate.proto;
declaration so that the PHP stubs don't initialize theinitOnce
hook.The text was updated successfully, but these errors were encountered: