Conversation
|
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
Renames oddly formatted protobuf repeated field names (i_pv4s / i_pv6s) to ipv4s / ipv6s in the Highway v1/v2 schemas to improve naming consistency across generated APIs.
Changes:
- Rename
i_pv4s→ipv4sinNTHighwayNetwork(v1 + v2). - Rename
i_pv4s/i_pv6s→ipv4s/ipv6sin rich media response message shapes (v1 + v2).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| proto/service/highway/v2/highway.proto | Renames IPv4/IPv6 repeated field identifiers in multiple messages for v2 schema consistency. |
| proto/service/highway/v1/ntv_2_rich_media_resp.proto | Renames IPv4/IPv6 repeated field identifiers in v1 rich media response messages. |
| proto/service/highway/v1/highway.proto | Renames IPv4 repeated field identifier in v1 NTHighwayNetwork. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| message NTHighwayNetwork { | ||
| repeated NTHighwayIPv4 i_pv4s = 1; | ||
| repeated NTHighwayIPv4 ipv4s = 1; |
There was a problem hiding this comment.
As with the v2 schema: this rename is wire-compatible but changes the generated accessor/JSON field name. Consider reserving the old name (i_pv4s) inside NTHighwayNetwork to prevent it from being reused in a future change.
| repeated NTHighwayIPv4 ipv4s = 1; | |
| repeated NTHighwayIPv4 ipv4s = 1; | |
| reserved "i_pv4s"; |
| @@ -68,7 +68,7 @@ message NTHighwayHash { | |||
| } | |||
|
|
|||
| message NTHighwayNetwork { | |||
There was a problem hiding this comment.
This field rename keeps the wire number (1) but changes the generated accessor name and the default JSON field name. To avoid accidental reintroduction of the old name later, consider reserving the previous field name (e.g., reserved "i_pv4s";) within this message (and use json_name only if you need to preserve the old JSON name for compatibility).
| message NTHighwayNetwork { | |
| message NTHighwayNetwork { | |
| reserved "i_pv4s"; |
| repeated IPv4 ipv4s = 4; | ||
| repeated IPv6 ipv6s = 5; |
There was a problem hiding this comment.
Renaming these fields changes generated API/JSON names even though the field numbers stay the same. Consider reserving the old names (i_pv4s, i_pv6s) inside DownloadInfo to prevent reuse and to make the compatibility intent explicit (and set json_name only if you must preserve the previous JSON keys).
| repeated IPv4 ipv4s = 4; | |
| repeated IPv6 ipv6s = 5; | |
| reserved "i_pv4s", "i_pv6s"; | |
| repeated IPv4 ipv4s = 4 [json_name = "i_pv4s"]; | |
| repeated IPv6 ipv6s = 5 [json_name = "i_pv6s"]; |
| message UploadResp { | ||
| optional string u_key = 1; | ||
| optional uint32 u_key_ttl_second = 2; | ||
| repeated IPv4 i_pv4s = 3; | ||
| repeated IPv6 i_pv6s = 4; | ||
| repeated IPv4 ipv4s = 3; | ||
| repeated IPv6 ipv6s = 4; |
There was a problem hiding this comment.
Same compatibility concern here: the rename changes generated API/JSON names. Please consider reserving i_pv4s/i_pv6s within UploadResp to prevent the old identifiers from being reused in future schema edits.
| optional uint32 sub_type = 1; | ||
| optional string u_key = 2; | ||
| optional uint32 u_key_ttl_second = 3; | ||
| repeated IPv4 i_pv4s = 4; | ||
| repeated IPv6 i_pv6s = 5; | ||
| repeated IPv4 ipv4s = 4; | ||
| repeated IPv6 ipv6s = 5; |
There was a problem hiding this comment.
Same as above: consider reserving the previous field names (i_pv4s, i_pv6s) within SubFileInfo after this rename, since the wire numbers remain but the public/generated names change.
| message DownloadInfo { | ||
| string domain = 1; | ||
| string url_path = 2; | ||
| uint32 https_port = 3; | ||
| repeated IPv4 i_pv4s = 4; | ||
| repeated IPv6 i_pv6s = 5; | ||
| repeated IPv4 ipv4s = 4; | ||
| repeated IPv6 ipv6s = 5; |
There was a problem hiding this comment.
This field rename preserves the wire number but changes generated API/JSON names. Consider reserving the old identifiers (i_pv4s, i_pv6s) within DownloadInfo to avoid accidental reuse and to clarify compatibility intent (and use json_name only if you need to keep the old JSON keys).
| message UploadResp { | ||
| optional string u_key = 1; | ||
| uint32 u_key_ttl_second = 2; | ||
| repeated IPv4 i_pv4s = 3; | ||
| repeated IPv6 i_pv6s = 4; | ||
| repeated IPv4 ipv4s = 3; | ||
| repeated IPv6 ipv6s = 4; |
There was a problem hiding this comment.
Same compatibility issue: renaming these fields changes generated API/JSON names. Consider reserving i_pv4s/i_pv6s inside UploadResp so the old names can’t be reused in later schema changes.
| uint32 sub_type = 1; | ||
| string u_key = 2; | ||
| uint32 u_key_ttl_second = 3; | ||
| repeated IPv4 i_pv4s = 4; | ||
| repeated IPv6 i_pv6s = 5; | ||
| repeated IPv4 ipv4s = 4; | ||
| repeated IPv6 ipv6s = 5; |
There was a problem hiding this comment.
Same here: the field numbers stay the same, but the rename changes generated API/JSON names. Consider reserving the old field names (i_pv4s, i_pv6s) within SubFileInfo to prevent accidental reuse later.
No description provided.