Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clippy fixes #1466

Merged
merged 5 commits into from
Dec 10, 2022
Merged

Clippy fixes #1466

merged 5 commits into from
Dec 10, 2022

Conversation

StygianLightning
Copy link
Contributor

I checked the examples and found a typo in one of the examples and an allowed lint as a workaround to a bug that's since been fixed. The other commits fix some clippy lints that show up on nightly.

djc
djc previously approved these changes Dec 7, 2022
Copy link
Member

@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.

Nice, thanks!

@StygianLightning
Copy link
Contributor Author

No worries!
There's one thing I'm not sure about, and that's the use of these allow annotations:

#[allow(clippy::unnecessary_cast)] // self.hdr.msg_controllen defined as size_t

I thought size_t was platform specific, but I looked it up earlier, and it's defined unconditionally as a type alias for usize, so maybe it would make sense to remove these annotations and the cast.

@Ralith
Copy link
Collaborator

Ralith commented Dec 8, 2022

Try it and see what CI thinks?

djc
djc previously approved these changes Dec 9, 2022
@StygianLightning
Copy link
Contributor Author

Huh, Mac doesn't like that for some reason. Gonna fix it tonight.

@StygianLightning
Copy link
Contributor Author

I fixed the three instances that failed on macOS and reduced the scope of the two #[allow] annotations from my original commit.

Copy link
Collaborator

@Ralith Ralith left a comment

Choose a reason for hiding this comment

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

Thanks!

@Ralith Ralith merged commit 389d7ce into quinn-rs:main Dec 10, 2022
@djc djc mentioned this pull request May 8, 2023
3 tasks
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.

3 participants