Skip to content

Conversation

michaelsproul
Copy link
Member

Proposed Changes

Fix Clippy lints enabled by default in Rust 1.73.0, released today.

@michaelsproul michaelsproul added ready-for-review The code is ready for review low-hanging-fruit Easy to resolve, get it before someone else does! labels Oct 5, 2023
Comment on lines +339 to +341
self.score()
.partial_cmp(&other.score())
.unwrap_or(std::cmp::Ordering::Equal)
Copy link
Member Author

Choose a reason for hiding this comment

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

Clippy thought the old impl was pretty suspicious.

The effect of Clippy's suggested change is that partial_ord will never return None for incomparable values (like NaN). I think this is OK, as we shouldn't be encountering this anyway (fingers crossed).

Copy link
Member

Choose a reason for hiding this comment

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

It doesn't look like there are any other scenarios (other than comparing NaN) where f64::partial_cmp would return None, so this is probably fine 🤞

Copy link
Member

@jimmygchen jimmygchen left a comment

Choose a reason for hiding this comment

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

Looks good! I've looked into the clippy suggestions as well, I think this change looks fine.

@jimmygchen jimmygchen added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Oct 6, 2023
@jimmygchen
Copy link
Member

Thanks!

bors r+

bors bot pushed a commit that referenced this pull request Oct 6, 2023
## Proposed Changes

Fix Clippy lints enabled by default in Rust 1.73.0, released today.
@bors
Copy link

bors bot commented Oct 6, 2023

Pull request successfully merged into unstable.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot changed the title Address Clippy 1.73 lints [Merged by Bors] - Address Clippy 1.73 lints Oct 6, 2023
@bors bors bot closed this Oct 6, 2023
@michaelsproul michaelsproul deleted the clippy-1.73 branch October 6, 2023 03:49
pawanjay176 pushed a commit that referenced this pull request Oct 6, 2023
* Address Clippy 1.73 lints (#4809)

## Proposed Changes

Fix Clippy lints enabled by default in Rust 1.73.0, released today.

* Address Clippy 1.73 lints.

---------

Co-authored-by: Michael Sproul <[email protected]>
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
## Proposed Changes

Fix Clippy lints enabled by default in Rust 1.73.0, released today.
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
## Proposed Changes

Fix Clippy lints enabled by default in Rust 1.73.0, released today.
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
## Proposed Changes

Fix Clippy lints enabled by default in Rust 1.73.0, released today.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
low-hanging-fruit Easy to resolve, get it before someone else does! ready-for-merge This PR is ready to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants