-
Notifications
You must be signed in to change notification settings - Fork 26
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
chore: add lint to p2p_sync #1841
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1841 +/- ##
===========================================
- Coverage 40.10% 18.49% -21.62%
===========================================
Files 26 100 +74
Lines 1895 12236 +10341
Branches 1895 12236 +10341
===========================================
+ Hits 760 2263 +1503
- Misses 1100 9546 +8446
- Partials 35 427 +392 ☔ View full report in Codecov by Sentry. |
2e7016b
to
5066da1
Compare
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.
Reviewable status: 0 of 4 files reviewed, all discussions resolved (waiting on @elintul)
crates/papyrus_p2p_sync/Cargo.toml
line 47 at r1 (raw file):
[lints] workspace = true
Removed, there are even more changes which I'll add in a different PR along with the lints, this is already too big.
Code quote:
[lints]
workspace = true
5066da1
to
fe47725
Compare
d48b96b
to
d06c9d6
Compare
0c14542
to
82f9571
Compare
Remove `as` usage, before adding workspace.lint support. Part 1 out of 2 for this crate. Context: Lior banned `as` repo-wide, unless absolutely necessary. Nearly all as uses can be replaced with [Try]From which doesn't have implicit coercions like as (we encountered several bugs due to these coercions). Motivation: we are standardizing lints across the repo and CI, instead of each crate having separate sets of lints. Changes: 1. header/state_diff: added clippy ignore to `gauge!` metric usage which requires f64. This is probably unnecessary since metrics used are int values, but fixing this is out of scope, and there are no other sensible conversions from unsigned to floats. 2. test.rs: most usages of these two vars were for usize, and only a couple needed u64, so i flipped the usages.
82f9571
to
620ad09
Compare
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.
Reviewed 2 of 4 files at r1, 1 of 1 files at r3, all commit messages.
Reviewable status: 3 of 4 files reviewed, all discussions resolved (waiting on @elintul)
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.
Reviewed 1 of 1 files at r2.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @elintul)
Lior banned
as
repo-wide, unless absolutely necessary.Nearly all as uses can be replaced with [Try]From which doesn't have implicit coercions like as (we encountered several bugs due to these coercions).
Motivation: we are standardizing lints across the repo and CI, instead of each crate having separate sets of lints.
Changes:
gauge!
metric usage which requires f64. This is probably unnecessary since metrics used are int values, but fixing this is out of scope, and there are no other sensible conversions from unsigned to floats.