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 #868

Merged
merged 6 commits into from
May 11, 2021
Merged

Clippy fixes #868

merged 6 commits into from
May 11, 2021

Conversation

greg-szabo
Copy link
Member

@greg-szabo greg-szabo commented Apr 22, 2021

Closes #406 and preferably #193 . (Todo: decide if this closes #193 based on discussion there.)

What does this do?

  • It keeps using clippy-check while addressing the issues mentioned in Fail on Clippy errors in CI #406 and Explore clippy rules and apply quality of life recommendations #193.

  • It turns the warnings in clippy-check into errors.

  • It clarifies what to look for:

    • The name of the task is now clippy-json-output to indicate that this is not the execution you want to check. (This is the task that executes clippy and produces a JSON output for parsing.)
    • The task creates a new clippy-results GitHub Action that renders the JSON output into a nice web view. (This is what should be checked and referred to.)
  • Now that all warnings are errors, I had to fix clippy warnings (turned errors) described here and here

  • For some reason the clippy-results task got created in the "Test" section in the latest run while it was created in the "Rust" section before (where the original trigger task is). This might be some internal GitHub Actions stuff, I consider it a harmless side-effect.

  • Referenced an issue explaining the need for the change

  • Updated all relevant documentation in docs

  • Updated all code comments where relevant

  • Wrote tests

  • Updated CHANGELOG.md

@codecov-commenter
Copy link

codecov-commenter commented Apr 22, 2021

Codecov Report

Merging #868 (aca24b2) into master (4fa065b) will decrease coverage by 0.0%.
The diff coverage is 100.0%.

❗ Current head aca24b2 differs from pull request most recent head 7929932. Consider uploading reports for the commit 7929932 to get more accurate results
Impacted file tree graph

@@           Coverage Diff            @@
##           master    #868     +/-   ##
========================================
- Coverage    67.0%   67.0%   -0.1%     
========================================
  Files         212     212             
  Lines       16771   16771             
========================================
- Hits        11244   11243      -1     
- Misses       5527    5528      +1     
Impacted Files Coverage Δ
proto/src/serializers/part_set_header_total.rs 100.0% <ø> (ø)
light-client-js/src/lib.rs 6.6% <100.0%> (ø)
rpc/src/endpoint/blockchain.rs 40.0% <0.0%> (-60.0%) ⬇️
rpc/src/endpoint/validators.rs 20.0% <0.0%> (-30.0%) ⬇️
testgen/src/header.rs 78.1% <0.0%> (-0.7%) ⬇️
light-client/src/types.rs 30.1% <0.0%> (-0.6%) ⬇️
tendermint/src/block/commit_sig.rs 79.5% <0.0%> (+2.2%) ⬆️
rpc/src/endpoint/abci_query.rs 21.4% <0.0%> (+3.7%) ⬆️
rpc/src/endpoint/broadcast/tx_async.rs 100.0% <0.0%> (+60.0%) ⬆️

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 4fa065b...7929932. Read the comment docs.

@greg-szabo greg-szabo changed the title [WIP] Clippy fixes Clippy fixes Apr 22, 2021
Copy link
Contributor

@thanethomson thanethomson left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@thanethomson thanethomson merged commit d910417 into master May 11, 2021
@thanethomson thanethomson deleted the greg/clippyyy branch May 11, 2021 15:45
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.

Fail on Clippy errors in CI Explore clippy rules and apply quality of life recommendations
3 participants