fix(buf): generate C# service clients from aggregate template - #65
Open
mikemcdougall wants to merge 1 commit into
Open
fix(buf): generate C# service clients from aggregate template#65mikemcdougall wants to merge 1 commit into
mikemcdougall wants to merge 1 commit into
Conversation
The aggregate buf.gen.yaml C# block listed only protocolbuffers/csharp and was missing the grpc/csharp plugin, even though its own comment claims it "matches buf.gen.csharp.yaml". As a result (the command docs/getting-started.md tells C# users to run) emitted message/enum types but no *ServiceClient stubs, so the Step 5 sample (new FeatureService.FeatureServiceClient(channel) + QueryFeaturesAsync) would not compile against the aggregate output. Every other language pairs its protobuf plugin with its gRPC plugin; C# was the only gap. Add grpc/csharp:v1.81.1 to the aggregate template (pinned to match buf.gen.csharp.yaml) so the canonical template produces the documented service clients. Verified: buf generate now emits namespace Geospatial.V1 plus 14 *ServiceClient classes. Also correct AGENTS.md, which described a C# "base namespace GeospatialGrpc". No base_namespace override is (or can be) set: managed mode derives Geospatial.V1 from the proto package, and a base_namespace that is not a prefix of that namespace makes the csharp plugin hard-fail. The stale claim would lead a contributor to add base_namespace=GeospatialGrpc and break C# generation. Findings: buf.gen.yaml:16 (missing grpc/csharp), AGENTS.md:115 (wrong namespace).
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two coupled C# code-generation defects in the canonical templates/docs, found in the round-5 release audit.
1. Aggregate
buf.gen.yamlomits thegrpc/csharpplugin (S2)buf.gen.yaml:16— the aggregate C# block listed onlyprotocolbuffers/csharpand was missinggrpc/csharp, even though its own comment asserts it "matches buf.gen.csharp.yaml and the published NuGet package." Every other language pairs its protobuf plugin with its gRPC plugin (Go, Java, Python); C# was the lone gap.Consequence:
buf generate— the exact commanddocs/getting-started.mdStep 3 tells C# users to run — emitted message/enum types but no*ServiceClientstubs. The Step 5 sample (new FeatureService.FeatureServiceClient(channel)+QueryFeaturesAsync) would not compile against that output. The NuGet package is unaffected (it usesGrpc.ToolswithGrpcServices=Both), so only the documented local-codegen C# path was broken, and CI's per-languagegeneratejob usesbuf.gen.csharp.yaml— so this landed green.Fix: add
grpc/csharp:v1.81.1(pinned to matchbuf.gen.csharp.yaml) to the aggregate template.2.
AGENTS.mddocuments a wrong/breaking C# base namespace (S2)AGENTS.md:115claimed generation uses "C# base namespaceGeospatialGrpc". Nobase_namespaceoverride is — or can be — set: managed mode derivesGeospatial.V1from the proto package, and abase_namespacethat is not a prefix of that namespace makes the csharp plugin hard-fail. All real consumers (examples/dotnet/Program.cs, getting-started, the smoke test) useGeospatial.V1. A contributor trusting AGENTS.md and addingbase_namespace=GeospatialGrpcwould break C# generation. Corrected the text to matchbuf.gen.yaml's own comment.Verification
Ran
buf generatewith the fixed aggregate C# block:Geospatial.V1(matches all consumers)*ServiceClientclasses generated (FeatureServiceClient,FormServiceClient, etc.)Empirically confirmed the inverse: setting
base_namespace=GeospatialGrpcfails withNamespace Geospatial.V1 is not a prefix namespace of base namespace GeospatialGrpc.Findings addressed
buf.gen.yaml:16— aggregate template missing grpc/csharp pluginAGENTS.md:115— stale/incorrect C# base-namespace claim