Skip to content

refactor(toolchain/names): replace toolchain_sort() with ToolchainName's Ord instance#3892

Merged
rami3l merged 3 commits intorust-lang:masterfrom
rami3l:refactor/toolchain-name-sort
Jun 19, 2024
Merged

refactor(toolchain/names): replace toolchain_sort() with ToolchainName's Ord instance#3892
rami3l merged 3 commits intorust-lang:masterfrom
rami3l:refactor/toolchain-name-sort

Conversation

@rami3l
Copy link
Copy Markdown
Member

@rami3l rami3l commented Jun 18, 2024

Closes #3884.

This new implementation also tries to correct the handling of X.Y, X.Y-beta.W and X.Y.Z-beta.W versions' ordering by abusing semver::Comparator's FromStr implementation.

@rami3l rami3l force-pushed the refactor/toolchain-name-sort branch from f738c39 to e147d73 Compare June 18, 2024 12:34
@rami3l rami3l marked this pull request as draft June 18, 2024 12:41
Copy link
Copy Markdown
Contributor

@djc djc left a comment

Choose a reason for hiding this comment

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

This looks great! I had started in the same direction but was a bit afraid of the time needed to get it right -- but would be awesome if you figure it out.

Left some early feedback.

Comment thread src/dist/mod.rs Outdated
Comment thread src/dist/mod.rs Outdated
Comment thread src/dist/mod.rs
Comment thread src/dist/mod.rs Outdated
Comment thread src/dist/mod.rs Outdated
Comment thread src/dist/mod.rs Outdated
@rami3l rami3l force-pushed the refactor/toolchain-name-sort branch 4 times, most recently from 171e53a to 3cb64d8 Compare June 19, 2024 03:55
@rami3l rami3l marked this pull request as ready for review June 19, 2024 04:34
@rami3l rami3l requested a review from djc June 19, 2024 04:34
@rami3l rami3l changed the title refactor(toolchain/names): replace toolchain_sort with ToolchainName's Ord instance refactor(toolchain/names): replace toolchain_sort() with ToolchainName's Ord instance Jun 19, 2024
@rami3l rami3l force-pushed the refactor/toolchain-name-sort branch from 3cb64d8 to 6cdf0b0 Compare June 19, 2024 05:36
Copy link
Copy Markdown
Contributor

@djc djc left a comment

Choose a reason for hiding this comment

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

Awesome!

@rami3l rami3l added this pull request to the merge queue Jun 19, 2024
Merged via the queue into rust-lang:master with commit 8ca641f Jun 19, 2024
@rami3l rami3l deleted the refactor/toolchain-name-sort branch June 19, 2024 11:13
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.

Consider getting rid of toolchain_sort()

2 participants