-
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: starknet API add lints #1853
Conversation
1e7d462
to
62c4ff8
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1853 +/- ##
==========================================
+ Coverage 40.10% 49.98% +9.87%
==========================================
Files 26 25 -1
Lines 1895 2573 +678
Branches 1895 2573 +678
==========================================
+ Hits 760 1286 +526
- Misses 1100 1222 +122
- Partials 35 65 +30 ☔ View full report in Codecov by Sentry. |
58c6382
to
7648096
Compare
7648096
to
57d1c0c
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 3 of 3 files at r2, all commit messages.
Reviewable status: 3 of 5 files reviewed, 1 unresolved discussion (waiting on @giladchase and @noaov1)
a discussion (no related file):
why are there changes to papyrus rpc here?
62c4ff8
to
c9abdaa
Compare
57d1c0c
to
2d946fe
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: 3 of 5 files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware and @noaov1)
a discussion (no related file):
Previously, dorimedini-starkware wrote…
why are there changes to papyrus rpc here?
Needed rebase, Done.
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, 2 of 2 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @noaov1)
c9abdaa
to
d575cbe
Compare
2d946fe
to
32faf44
Compare
Benchmark movements: |
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: complete! all files reviewed, all discussions resolved (waiting on @noaov1)
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. Notes: changes to docs are due to cargo doc now running on changed files
32faf44
to
f33ef83
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 4 of 4 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @noaov1)
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. Notes: changes to docs are due to cargo doc now running on changed files Co-Authored-By: Gilad Chase <gilad@starkware.com>
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.