Skip to content

[api] icmp/icmp6icmp_v4/icmp_v6#10544

Draft
david-crespo wants to merge 2 commits into
mainfrom
rename-icmp-protocols
Draft

[api] icmp/icmp6icmp_v4/icmp_v6#10544
david-crespo wants to merge 2 commits into
mainfrom
rename-icmp-protocols

Conversation

@david-crespo
Copy link
Copy Markdown
Contributor

@david-crespo david-crespo commented Jun 3, 2026

We're already calling them ICMPv4 and ICMPv6 in the console as of oxidecomputer/console#3212. @taspelund agreed with me that it would be ideal if the API matched that.

This is a fully robot-authored attempt at the rename. Leaving draft because I will need to review it. I went with IcmpV4 and IcmpV6 in Rust, which by default becomes icmp_v4 and icmp_v6 in the JSON. That seems like the least bad option, but it would be easy to change. It's a lot of line additions, but it's all versioning boilerplate and tests. It looks pretty good at a glance.

"type": "string",
"enum": [
"icmp6"
"icmp_v6"
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of this boilerplate just for this...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant