-
Notifications
You must be signed in to change notification settings - Fork 21
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(sync): add lints to sync #1847
Conversation
cf9d901
to
1b3f257
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1847 +/- ##
===========================================
- Coverage 40.10% 24.63% -15.48%
===========================================
Files 26 79 +53
Lines 1895 8924 +7029
Branches 1895 8924 +7029
===========================================
+ Hits 760 2198 +1438
- Misses 1100 6414 +5314
- Partials 35 312 +277 ☔ View full report in Codecov by Sentry. |
1b3f257
to
b7e68f2
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 1 of 2 files at r1, all commit messages.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @elintul and @giladchase)
crates/papyrus_sync/src/lib.rs
line 0 at r1 (raw file):
can you move the #[allow]
s to above the problematic lines, instead of annotating blocks of code?
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: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware and @elintul)
crates/papyrus_sync/src/lib.rs
line at r1 (raw file):
Previously, dorimedini-starkware wrote…
can you move the
#[allow]
s to above the problematic lines, instead of annotating blocks of code?
I wish 🥲
The problematic lines here are gauge!(...)
, and #[allow(clippy
doesn't work on macros, only on enclosing items.
So in all the places that included this macro I added the ignore at the closest enclosing non-macro parent.
In parallel I'm figuring out how I can test out the output of these metrics and see if switching from gauge!
to counter!
is equivalent output-wise (the latter take integer values and the former takes floats, even though our metrics are integers)
61307b8
to
67bb3fa
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 1 of 2 files at r1.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @elintul)
b230c2d
to
a55bfb3
Compare
Lior banned `as` repo-wide, unless absolutely necessary.
b7e68f2
to
295c5c1
Compare
Lior banned
as
repo-wide, unless absolutely necessary.