Skip to content

Make clippy a bit happier#104

Merged
sebcrozet merged 1 commit into
dimforge:masterfrom
EmbarkStudios:clippy-fixes
Feb 4, 2021
Merged

Make clippy a bit happier#104
sebcrozet merged 1 commit into
dimforge:masterfrom
EmbarkStudios:clippy-fixes

Conversation

@emilk

@emilk emilk commented Feb 4, 2021

Copy link
Copy Markdown
Contributor

I did a quick pass fixing some things that clippy complained about. Nothing major. I there is anything you disagree with I am happy to revert.

let axis2 = position2 * self.local_axis2;
let delta_rot =
Rotation::rotation_between_axis(&axis1, &axis2).unwrap_or(Rotation::identity());
Rotation::rotation_between_axis(&axis1, &axis2).unwrap_or_else(Rotation::identity);

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.

maybe .unwrap_or_default() would be even better?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I like the more explicit .unwrap_or_else(Rotation::identity) better. The docs about Default states:

Sometimes, you want to fall back to some kind of default value, and don't particularly care what it is [...]

Since we do care about the fact we get the identity, it feels like Default isn't appropriate.

@sebcrozet

Copy link
Copy Markdown
Member

Everything is OK. Thanks!

@sebcrozet
sebcrozet merged commit a272f4c into dimforge:master Feb 4, 2021
jamespohalloran pushed a commit to jamespohalloran/rapier that referenced this pull request Apr 26, 2025
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