command: model the GetRateTariffResponse reply payload (field 12)#12
Merged
Conversation
Field 12 was left reserved in #10 pending a decision on whether to publish a rate-tariff type from an app-only namespace this package doesn't otherwise model. The captain decided to publish: the same tariff document is already writable via SetRateTariffRequest (tag 55) and readable via GetRateTariffRequest (tag 56), so the read reply is worth modelling too. Model GetRateTariffResponse by mirroring the tariff document already declared for SetRateTariffRequest (Seasons/Tariff at tags 13/14), based on our own observations and contributions from the community, rather than re-declaring a separate namespace for a single reused document shape. Un-reserving field 12 trips buf's RESERVED_MESSAGE_NO_DELETE breaking check even though it's a wire-compatible addition, so except it repo-wide in buf.yaml - the same fields-17/19 follow-up will hit it too.
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.
Intent
GetRateTariffResponse) was deliberately leftreservedin command: fill in the CarServer.Response reply-payload oneof #10 pending a decision on whether to publish a rate-tariff type from an app-only namespace this package doesn't otherwise model. That decision is now made: publish it.SetRateTariffRequest(VehicleActiontag 55) and readable viaGetRateTariffRequest(tag 56), both already modelled in this package - so the read reply is worth completing too.GetRateTariffResponseby reusingSetRateTariffRequest.Seasons/SetRateTariffRequest.Tariffrather than re-declaring a separate namespace for the same document shape, based on our own observations and contributions from the community.RESERVED_MESSAGE_NO_DELETEbreaking check even though it's a wire-compatible addition (nothing on the wire ever actually used field 12 before). Excepted the rule repo-wide inbuf.yamlrather than per-PR, since the fields-17/19 follow-up will hit the same thing.centerdisplay.server) and 19 (webrtc_comms) stay reserved - separate follow-up, different domains.Response.getRateTariffResponse.{seasons,tariff}in both TypeScript and Python.buf lint,buf breaking, upstream coverage gate, and the reply-coverage gate are all green.