-
Notifications
You must be signed in to change notification settings - Fork 490
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
Added *Assign
ops and Depracated *Eq
ops.
#5679
Conversation
ef9ed72
to
49a7db0
Compare
25be00b
to
0062671
Compare
0062671
to
66ea8fd
Compare
49a7db0
to
97e4ee1
Compare
97e4ee1
to
7584714
Compare
06e3ecf
to
c295e8d
Compare
7584714
to
b1b4e11
Compare
c295e8d
to
df2ec88
Compare
b1b4e11
to
df418b0
Compare
df418b0
to
4f43e9b
Compare
4f43e9b
to
798d5cd
Compare
e714019
to
f07f5dc
Compare
798d5cd
to
3cf96da
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 9 of 12 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: 10 of 12 files reviewed, 2 unresolved discussions (waiting on @orizi)
corelib/src/byte_array.cairo
line 352 at r2 (raw file):
} #[feature("deprecated-op-assign-traits")] impl ByteArrayAddEq of core::traits::AddEq<ByteArray> {
For consistency.
Suggestion:
mpl ByteArrayAddEq of AddEq<ByteArray> {
corelib/src/ops.cairo
line 4 at r2 (raw file):
pub use index::{Index, IndexView}; mod arith;
It's not the Rust equivalent name right? Consider opening it to discussion.
Code quote:
mod arith;
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: 10 of 12 files reviewed, 2 unresolved discussions (waiting on @gilbens-starkware)
corelib/src/ops.cairo
line 4 at r2 (raw file):
Previously, gilbens-starkware (Gil Ben-Shachar) wrote…
It's not the Rust equivalent name right? Consider opening it to discussion.
Exact same as rust, and more importantly, not pub
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: 10 of 12 files reviewed, 2 unresolved discussions (waiting on @gilbens-starkware)
corelib/src/byte_array.cairo
line 352 at r2 (raw file):
Previously, gilbens-starkware (Gil Ben-Shachar) wrote…
For consistency.
don't think it is required - as it should be removed from the prelude as well.
(this will just be added to be removed)
commit-id:2f5d0193
3cf96da
to
293c82e
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 11 of 12 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @gilbens-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 2 of 12 files at r1.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @orizi)
corelib/src/byte_array.cairo
line 352 at r2 (raw file):
Previously, orizi wrote…
don't think it is required - as it should be removed from the prelude as well.
(this will just be added to be removed)
You didn't add the path in all other places, but not really important.
Stack:
*Assign
ops and Depracated*Eq
ops. #5679 ⬅This change is