-
Notifications
You must be signed in to change notification settings - Fork 31
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(cairo_native): make the starknet-native-compile crate a part of the workspace #3021
base: main-v0.13.4
Are you sure you want to change the base?
Conversation
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 5 of 5 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @avi-starkware, @meship-starkware, and @Yoni-Starkware)
Cargo.toml
line 60 at r1 (raw file):
] exclude = ["crates/bin/starknet-native-compile"]
What does it mean?
Code quote:
exclude = ["crates/bin/starknet-native-compile"]
crates/bin/starknet-native-compile/src/utils.rs
line 1 at r1 (raw file):
#[cfg(feature = "cairo_native")]
Is it necessary, considering the file already has the feature flag above its declaration? 🤔
Code quote:
#[cfg(feature = "cairo_native")]
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: all files reviewed, 3 unresolved discussions (waiting on @avi-starkware, @meship-starkware, and @Yoni-Starkware)
crates/bin/starknet-native-compile/Cargo.toml
line 11 at r1 (raw file):
[features] cairo_native = [
How do you make sure that the starknet_sierra_compile
crate uses this crate with the cairo_native
feature flag?
Code quote:
cairo_native = [
Benchmark movements: |
e03856d
to
7a03873
Compare
Previously, noaov1 (Noa Oved) wrote…
starknet_sierra_compile does not depend on this crate directly it builds it via |
Previously, noaov1 (Noa Oved) wrote…
It just means that the crate is not part of the workspace and will not be compiled automatically when running |
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.
Reviewed 5 of 5 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @avi-starkware, @noaov1, and @Yoni-Starkware)
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, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @avi-starkware and @Yoni-Starkware)
Previously, noaov1 (Noa Oved) wrote…
If we don't put this over each import that is only used with the feature flag, we will get an |
952e380
to
a88df6a
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 r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Yoni-Starkware)
7220231
to
6eb4b19
Compare
6eb4b19
to
8d48934
Compare
No description provided.