Skip to content

Rename SwitchLocation -> SwitchSlot#10002

Merged
jgallagher merged 2 commits intomainfrom
john/switch-location-to-switch-slot
Mar 10, 2026
Merged

Rename SwitchLocation -> SwitchSlot#10002
jgallagher merged 2 commits intomainfrom
john/switch-location-to-switch-slot

Conversation

@jgallagher
Copy link
Copy Markdown
Contributor

@jgallagher jgallagher commented Mar 9, 2026

Followup from #9984 which added the database-level DbSwitchSlot enum, this renames our SwitchLocation enum to SwitchSlot (and renames local variables in many spots, although I'm sure I missed a few). This is a combination of "consistency between Rust types and db types" and "free up the name SwitchLocation which implies both a rack ID + a switch slot".

This bumps the sled-agent and external API versions, but in a wire-compatible way (just a type rename).

Copy link
Copy Markdown
Collaborator

@smklein smklein left a comment

Choose a reason for hiding this comment

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

LGTM - i understand the most important part of this is changing the externally-facing names, and I think this PR already accomplishes that.

My feedback is basically all nits, but all my comments are about convergence of our many "internal" names for this field, which appear to vary between:

  • location
  • loc
  • slot
  • switch_slot

converging on one, as much as possible, would be nice. but I'll defer to your judgement on how much time to spend aligning that, since "database type names" and "API type names" are the ones that matter the most

@jgallagher
Copy link
Copy Markdown
Contributor Author

My feedback is basically all nits, but all my comments are about convergence of our many "internal" names for this field

Thanks, this is great - I agree we should make these line up as best we can. I'll grab all the ones you found.

@jgallagher jgallagher merged commit df724fa into main Mar 10, 2026
17 checks passed
@jgallagher jgallagher deleted the john/switch-location-to-switch-slot branch March 10, 2026 01:15
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.

2 participants