-
Notifications
You must be signed in to change notification settings - Fork 126
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 tests #122
Clippy tests #122
Conversation
Not sure how this can be done from the command-line, but VS Code brings all these up.
if client { | ||
hconn = Http3Connection::new(default_client(), 100, 100, None); | ||
neqo_trans_conn = default_server(); | ||
let (mut hconn, mut neqo_trans_conn) = if client { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like how clippy warns about this one. But I'm not in a mood to suppress the change, even if it isn't great.
@@ -1332,16 +1333,14 @@ mod tests { | |||
let events = neqo_trans_conn.events(); | |||
let mut stop_sending_event_found = false; | |||
for e in events { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a common pattern. I was uncertain about whether to change this to use Iterator::any, which would be better for looking for the event. But that would be lots of typing.
it seems |
I went with something a little broader than that. |
Oh, it does:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😮
👍
VS Code runs clippy on tests. I don't know how to do that from the command line (or else I would change our CI setup as well), but these are the problems I found and fixed.
The only thing I didn't fix were a couple of functions that were past the cognitive complexity limit.