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

Fixed some clippy warning #1801

Merged
merged 3 commits into from Oct 26, 2022
Merged

Fixed some clippy warning #1801

merged 3 commits into from Oct 26, 2022

Conversation

darnuria
Copy link
Contributor

Leaved the println! warnings.

crates/proto/src/rr/domain/usage.rs Show resolved Hide resolved
crates/proto/src/op/header.rs Outdated Show resolved Hide resolved
@darnuria
Copy link
Contributor Author

(have some more to fix i am on it)

crates/proto/src/op/header.rs Outdated Show resolved Hide resolved
crates/proto/src/op/message.rs Outdated Show resolved Hide resolved
@darnuria
Copy link
Contributor Author

darnuria commented Oct 16, 2022

Look like clippy is... misleading on this one, suppose it was warn based on regex not AST.

 use of `unwrap_or` followed by a function call
   --> crates/proto/src/https/https_client_stream.rs:164:22
    |
164 |                     .unwrap_or(Ok(crate::https::MIME_APPLICATION_DNS))?;
    |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| Ok(crate::https::MIME_APPLICATION_DNS))`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#or_fun_call
    = note: `-D clippy::or-fun-call` implied by `-D warnings`

@darnuria darnuria force-pushed the clippy/fix branch 4 times, most recently from c594250 to 0108d1d Compare October 16, 2022 20:00
@bluejekyll
Copy link
Member

So for the failing option, would you mind adding this to the lib.rs in at least proto? it could be added to all the allows though: clippy::bool_to_int_with_if in https://github.com/bluejekyll/trust-dns/blob/d4d065e49b0e4b05b51a13324b9e4a1c20d0bb13/crates/proto/src/lib.rs#L23-L26

crates/proto/src/op/header.rs Outdated Show resolved Hide resolved
@djc
Copy link
Collaborator

djc commented Oct 20, 2022

This fails cleanliness CI, maybe because the lint isn't available on the clippy version that we run in CI?

@darnuria
Copy link
Contributor Author

darnuria commented Oct 23, 2022

2022-10-18T23:58:41.8365550Z error: unknown lint: `clippy::bool_to_int_with_if`
2022-10-18T23:58:41.8366698Z   --> crates/proto/src/lib.rs:26:5
2022-10-18T23:58:41.8425708Z    |
2022-10-18T23:58:41.8426569Z 26 |     clippy::bool_to_int_with_if,
2022-10-18T23:58:41.8426831Z    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^
2022-10-18T23:58:41.8427042Z    |
2022-10-18T23:58:41.8428048Z    = note: `-D unknown-lints` implied by `-D warnings`

Oh sorry didn't found time to check CI, will investigate today. @djc right I were in the future for this one will revert the commit and keep it for a later day.

My version of clippy:
clippy 0.1.66 (8b0c05d 2022-10-07)

EDIT: found time to do it.

  • Rebased against origin/main
  • splited the cast from the if commit
  • Removed commit with allow, saved for a future day

@codecov
Copy link

codecov bot commented Oct 25, 2022

Codecov Report

Merging #1801 (0094249) into main (29eeef4) will not change coverage.
The diff coverage is 80.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1801   +/-   ##
=======================================
  Coverage   80.40%   80.40%           
=======================================
  Files         177      177           
  Lines       18242    18242           
=======================================
  Hits        14668    14668           
  Misses       3574     3574           
Impacted Files Coverage Δ
crates/proto/src/multicast/mdns_stream.rs 25.40% <50.00%> (ø)
crates/proto/src/rustls/tls_server.rs 88.88% <100.00%> (ø)
crates/proto/src/serialize/binary/bin_tests.rs 100.00% <100.00%> (ø)
crates/proto/src/xfer/dns_response.rs 96.14% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 29eeef4...0094249. Read the comment docs.

@djc djc merged commit 655a157 into hickory-dns:main Oct 26, 2022
@djc
Copy link
Collaborator

djc commented Oct 26, 2022

Thanks!

XOR-op added a commit to XOR-op/hickory-dns that referenced this pull request Jan 8, 2023
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.

None yet

3 participants