Skip to content

Standardize DType match arms to follow variant declaration order#7897

Open
connortsui20 wants to merge 1 commit into
developfrom
claude/standardize-dtype-match-order-Pr2NG
Open

Standardize DType match arms to follow variant declaration order#7897
connortsui20 wants to merge 1 commit into
developfrom
claude/standardize-dtype-match-order-Pr2NG

Conversation

@connortsui20
Copy link
Copy Markdown
Contributor

Reorder DType match arms across the codebase to match the order of the
DType enum declaration, and move Extension after Variant so that
Extension is the last variant in the enum (and in all match statements).

This is a purely cosmetic refactor with no functional changes:

  • The protobuf, flatbuffer, and serde wire formats are preserved (only the
    source order of match arms is changed, not the discriminant/tag values).
  • The public API surface is unchanged; public-api.lock files are
    alphabetical so they require no update.

Signed-off-by: Claude noreply@anthropic.com

Reorder `DType` match arms across the codebase to match the order of the
`DType` enum declaration, and move `Extension` after `Variant` so that
`Extension` is the last variant in the enum (and in all match statements).

This is a purely cosmetic refactor with no functional changes:
- The protobuf, flatbuffer, and serde wire formats are preserved (only the
  source order of match arms is changed, not the discriminant/tag values).
- The public API surface is unchanged; `public-api.lock` files are
  alphabetical so they require no update.

Signed-off-by: Claude <noreply@anthropic.com>
@connortsui20 connortsui20 enabled auto-merge (squash) May 12, 2026 13:16
@connortsui20 connortsui20 added the changelog/chore A trivial change label May 12, 2026
Copy link
Copy Markdown
Contributor

@robert3005 robert3005 left a comment

Choose a reason for hiding this comment

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

I don't think this is worth it. While I understand it makes more sense to be consistent you gain nothing without writing a rule that enforces this in perpetuity. I would be happy to merge a pr like this if it came with a clippy rule

@connortsui20
Copy link
Copy Markdown
Contributor Author

This doesn't have to be a rule though?

It is not a big deal if they are not in the same order, the issue is that with 11 (soon 12) variants we should remove any amount of overhead when trying to grok the code. It can be confusing when arms are in completely different orders, so why not just clean this up?

@robert3005
Copy link
Copy Markdown
Contributor

The claim I am making is that you're not removing any complexity, you're just moving it to suit your preferences. Removing complexity imho is automating this change but then there's complexity w.r.t. the calling code doing something special with certain variants

@connortsui20
Copy link
Copy Markdown
Contributor Author

I disagree that moving code around is simply to suit someone's preferences, if that were the case then we should just put everything in a single file (since it is just a preference that we break modules into files).

I don't understand why this is a crazy change given it is in an effort to make code more consistent and readable top to bottom (which is often what people new to a codebase will do).

I understand that it is not the ideal (the ideal would be that there is a lint where if a match statement has all possible arms handled it orders them), but to say that because the ideal cannot be automated we should just give up entirely is somewhat confusing to me.

There is a similar issue with some of our trait implementations where the order of methods is not always consistent with the trait definition and there is a rust analyzer action that fixes this, but it cannot be "automated". Are we really going to say that if someone decides to fix that we're not going to accept that change because there might be some drift later?

@robert3005
Copy link
Copy Markdown
Contributor

This is not a crazy change, this is an unnecessary change. I am not saying any of what you're implying. I am saying that you shouldn't merge pure cleanups without rules and you can merge cleanups with changes to the code you're touching

@connortsui20
Copy link
Copy Markdown
Contributor Author

The only reason I split this out was because @joseph-isaacs asked me to split it out of #7890 (which is now #7897)

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

Labels

changelog/chore A trivial change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants