-
Notifications
You must be signed in to change notification settings - Fork 25
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 with the lastest changes in the Blockifier repo #24
Conversation
3e6d766
to
af1775f
Compare
af1775f
to
fead13e
Compare
fead13e
to
8abb01b
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 25 files at r1.
Reviewable status: 3 of 25 files reviewed, 2 unresolved discussions (waiting on @dafnamatsry, @noaov1, and @TzahiTaub)
Cargo.toml
line 72 at r1 (raw file):
cairo-lang-sierra = "=2.7.0-rc.3" cairo-lang-starknet-classes = "2.7.0-rc.3" cairo-lang-utils = "2.7.0-rc.3"
this will potentially break other crates...
I would like to see the papyrus CI run successfully here.
Can you try adding a dummy comment in some papyrus file and pushing? so the papyrus CI runs with the new compiler?
(for papyrus CI to pass, this PR needs to be merged and the current (#24) PR should be rebased, but you can ignore the docker-build papyrus phases for now)
Code quote:
cairo-lang-casm = "2.7.0-rc.3"
cairo-lang-runner = "2.7.0-rc.3"
cairo-lang-sierra = "=2.7.0-rc.3"
cairo-lang-starknet-classes = "2.7.0-rc.3"
cairo-lang-utils = "2.7.0-rc.3"
Cargo.toml
line 119 at r1 (raw file):
os_info = "3.6.0" page_size = "0.6.0" papyrus_storage = "0.4.0-dev.4"
no - papyrus storage is a crate in this repo.
dependency should be via path, not version, in the (native_) blockifier cargo toml (not the root)
Code quote:
papyrus_storage = "0.4.0-dev.4"
8abb01b
to
5282a0e
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: 2 of 26 files reviewed, 2 unresolved discussions (waiting on @dafnamatsry, @dorimedini-starkware, and @noaov1)
Cargo.toml
line 119 at r1 (raw file):
Previously, dorimedini-starkware wrote…
no - papyrus storage is a crate in this repo.
dependency should be via path, not version, in the (native_) blockifier cargo toml (not the root)
Trying to remove this. I guess it shouldn't appear in here at all, I see every crate's toml declares it explicitly. Don't we want to define it once in here and make all others use workspace
settings for this crate?
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #24 +/- ##
==========================================
- Coverage 76.90% 76.88% -0.02%
==========================================
Files 267 267
Lines 31923 32011 +88
Branches 31923 32011 +88
==========================================
+ Hits 24550 24613 +63
- Misses 5179 5196 +17
- Partials 2194 2202 +8 ☔ View full report in Codecov by Sentry. |
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 r2, all commit messages.
Reviewable status: 3 of 26 files reviewed, 1 unresolved discussion (waiting on @dafnamatsry, @noaov1, and @TzahiTaub)
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 26 files reviewed, 1 unresolved discussion (waiting on @dafnamatsry, @noaov1, and @TzahiTaub)
Cargo.toml
line 72 at r1 (raw file):
Previously, dorimedini-starkware wrote…
this will potentially break other crates...
I would like to see the papyrus CI run successfully here.
Can you try adding a dummy comment in some papyrus file and pushing? so the papyrus CI runs with the new compiler?
(for papyrus CI to pass, this PR needs to be merged and the current (#24) PR should be rebased, but you can ignore the docker-build papyrus phases for now)
looks good
crates/papyrus_node/examples/get_transaction_hash.rs
line 96 at r2 (raw file):
} // Define a tuple struct to hold transaction type and version.
lol
Code quote:
.
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 26 files reviewed, 1 unresolved discussion (waiting on @dafnamatsry, @dorimedini-starkware, and @noaov1)
crates/papyrus_node/examples/get_transaction_hash.rs
line 96 at r2 (raw file):
Previously, dorimedini-starkware wrote…
lol
Don't want to force myself
5282a0e
to
f42b3cf
Compare
f42b3cf
to
c27bb8c
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 22 of 25 files at r1, 1 of 2 files at r2.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dafnamatsry and @noaov1)
chore: sync with the lastest changes in the Blockifier repo
This change is