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

Enable clippy in CI #4026

Merged
merged 9 commits into from
Nov 2, 2023
Merged

Enable clippy in CI #4026

merged 9 commits into from
Nov 2, 2023

Conversation

obycode
Copy link
Contributor

@obycode obycode commented Nov 1, 2023

Description

Applicable issues

Additional info (benefits, drawbacks, caveats)

Checklist

  • Test coverage for new or modified code paths
  • Changelog is updated
  • Required documentation changes (e.g., docs/rpc/openapi.yaml and rpc-endpoints.md for v2 endpoints, event-dispatcher.md for new events)
  • New clarity functions have corresponding PR in clarity-benchmarking repo
  • New integration test(s) added to bitcoin-tests.yml

@obycode
Copy link
Contributor Author

obycode commented Nov 1, 2023

@wileyj after this is merged, can we add this "Clippy Check" as a required check?

@obycode
Copy link
Contributor Author

obycode commented Nov 1, 2023

Just realized that's probably not the right place to add the clippy job in ci.yml, since that one's only activated on pushes to master. I'm not clear on where it needs to go.

Copy link

codecov bot commented Nov 1, 2023

Codecov Report

Merging #4026 (6315b17) into develop (3b37332) will decrease coverage by 58.16%.
Report is 1 commits behind head on develop.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##           develop    #4026       +/-   ##
============================================
- Coverage    58.31%    0.16%   -58.16%     
============================================
  Files            1      402      +401     
  Lines          571   287893   +287322     
============================================
+ Hits           333      469      +136     
- Misses         238   287424   +287186     
Files Coverage Δ
clarity/src/vm/ast/mod.rs 0.00% <ø> (ø)
clarity/src/vm/ast/parser/v1.rs 0.00% <ø> (ø)
clarity/src/vm/analysis/errors.rs 0.00% <0.00%> (ø)
...ty/src/vm/analysis/type_checker/v2_05/tests/mod.rs 0.00% <0.00%> (ø)
...ity/src/vm/analysis/type_checker/v2_1/tests/mod.rs 0.00% <0.00%> (ø)
stacks-common/src/util/log.rs 0.00% <0.00%> (ø)
stacks-common/src/util/macros.rs 0.00% <0.00%> (ø)
stacks-common/src/util/pipe.rs 0.00% <0.00%> (ø)
clarity/src/vm/ast/parser/v2/mod.rs 0.00% <0.00%> (ø)
stacks-common/src/types/net.rs 0.00% <0.00%> (ø)
... and 1 more

... and 390 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@obycode
Copy link
Contributor Author

obycode commented Nov 1, 2023

I reverted the changes to ci.yml, but I don't understand yet why this job doesn't actually run.

@obycode
Copy link
Contributor Author

obycode commented Nov 1, 2023

Ah, I'm an idiot:

    if: ${{ false }}

I'll try removing that.

Copy link
Contributor

@wileyj wileyj left a comment

Choose a reason for hiding this comment

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

Approving this so we have something in place for now, with the expectation that we'll want to reimplement what this remote workflow does since it is no longer maintained:
https://github.com/actions-rs/clippy-check

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