Conversation
…ansElem, and MarketFace messages
|
The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request).
|
There was a problem hiding this comment.
Pull request overview
This PR enhances the Elem protobuf by adding support for additional element types and introducing new message definitions to represent them.
Changes:
- Added
face,trans_elem, andmarket_facefields tomessage Elem. - Introduced new protobuf messages:
Face,TransElem, andMarketFace.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| optional int32 media_type = 9; | ||
| optional int32 image_width = 10; | ||
| optional int32 image_height = 11; | ||
| optional bytes mobileparam = 12; |
There was a problem hiding this comment.
mobileparam breaks the established snake_case pattern used by other fields in MarketFace (e.g., face_name, tab_id). Consider renaming it to mobile_param for consistency and clearer generated API naming.
| optional bytes mobileparam = 12; | |
| optional bytes mobile_param = 12; |
|
|
||
| message Face { | ||
| optional int32 index = 1; | ||
| optional bytes old = 2; |
There was a problem hiding this comment.
Face.buf uses tag 11, skipping tags 3..10. If this gap is intentional for wire-compatibility with an upstream schema, it would help to add reserved 3 to 10; (or a comment) to prevent accidental reuse later. If it's not intentional, consider using the next available sequential tag to reduce confusion.
| optional bytes old = 2; | |
| optional bytes old = 2; | |
| reserved 3 to 10; |
| message TransElem { | ||
| optional int32 elem_type = 1; |
There was a problem hiding this comment.
elem_type is a raw int32, which makes the set of supported values unclear in generated APIs. Consider introducing an enum for elem_type (even if incomplete) and adding a brief comment describing the encoding/format of elem_value for each type to make the contract self-describing.
| message TransElem { | |
| optional int32 elem_type = 1; | |
| enum TransElemType { | |
| // Default / unknown transport element type. When used, elem_value is opaque bytes. | |
| TRANS_ELEM_TYPE_UNSPECIFIED = 0; | |
| // Additional element types can be added here as they become known. | |
| } | |
| message TransElem { | |
| // Logical type of elem_value. See TransElemType for known values. | |
| optional TransElemType elem_type = 1; | |
| // Payload whose encoding depends on elem_type. For UNSPECIFIED or unknown types, | |
| // this field contains opaque bytes as defined by the upstream service. |
…ansElem, and MarketFace messages