adding a new rpc to a proto is a breaking api change

[carnott-snap]

While I can understand that sun-setting a field (read: reserved field) in a proto would lead to a breaking change in generated Go code, it confuses me why adding a new rpc would also cause a breaking API change. (Reproducer available upon request.)

[dfawley]

@carnott-snap this is #2318, fixed by golang/protobuf#785 (but it requires user action every time, unfortunately).

[carnott-snap]

Thanks for the reference, and my sincerest apologies for missing this when I searched for related issues.

Is this feature documented publicly? My colleagues and I have been using gRPC for a while and this recently caught us unawares.

[dfawley]

@carnott-snap not very well, no. I will be adding the usage of this to our examples, and updating any related documentation.

[carnott-snap]

Furthermore, could this be added to the docstrings of the service interfaces, like with the // Deprecated: Do not use. warnings? I am need to see what the stdlib does, because iirc there is not a spec as with deprecation, but I would think something like, // Unstable: Embed UnimplementedXxxServer into any structs implementing this interface., would go a long way to help.

[carnott-snap]

I also notice that this only generates UnimplementedXxxThing for the XxxServer. We frequently implement clients for testing, and I could see breaking tests being problematic too. Are you opposed to adding UnimplementedXxxClient?

[dfawley]

@carnott-snap for test clients, you can embed the XxxClient interface directly. It will panic if something newly added is used, but for a test this seems reasonable -- you need to implement it if you need to use it.

[dfawley]

@carnott-snap - regarding:

Furthermore, could this be added to the docstrings of the service interfaces, like with the // Deprecated: Do not use.

Something like that would make sense. If you have the cycles, this kind of change would be excellent as a contribution!

[carnott-snap]

@carnott-snap for test clients, you can embed the XxxClient interface directly. It will panic if something newly added is used, but for a test this seems reasonable -- you need to implement it if you need to use it.

I will try this out.

Something like that would make sense. If you have the cycles, this kind of change would be excellent as a contribution!

Yeah, let me see if I can make time, the change seems simple enough. Would you like me to spawn an issue to track and finalise the design?

[dfawley]

Yeah, let me see if I can make time, the change seems simple enough. Would you like me to spawn an issue to track and finalise the design?

Thanks. No, if you are only adding comments to the generated code, you can just send the PR and we can bikeshed on the precise desired wording in the review.