-
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
feat(blockifier): recompile Cairo1 in the CI #197
feat(blockifier): recompile Cairo1 in the CI #197
Conversation
e98af69
to
801ff61
Compare
9a950bf
to
25de71b
Compare
801ff61
to
8788493
Compare
25de71b
to
ca8d23b
Compare
8788493
to
6e3d149
Compare
ca8d23b
to
6799aa3
Compare
6e3d149
to
d5ef49f
Compare
6799aa3
to
dd8634f
Compare
d5ef49f
to
aafebad
Compare
dd8634f
to
9782b30
Compare
aafebad
to
16edf9a
Compare
9782b30
to
e6f7a3f
Compare
16edf9a
to
1f3a521
Compare
e6f7a3f
to
af5515a
Compare
1f3a521
to
97e4c2f
Compare
f282206
to
db0b683
Compare
5d3dbb0
to
afc8510
Compare
db0b683
to
fa2914a
Compare
afc8510
to
e87bd1c
Compare
fa2914a
to
9e7cac1
Compare
e87bd1c
to
9b0ad2b
Compare
9e7cac1
to
4993d6c
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 r10, 2 of 2 files at r11, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @aner-starkware, @AvivYossef-starkware, @dorimedini-starkware, @meship-starkware, and @noaov1)
.github/workflows/blockifier_compiled_cairo.yml
line 14 at r11 (raw file):
- 'crates/blockifier/feature_contracts/**' - 'crates/blockifier/src/test_utils/cairo_compile.rs' - 'crates/blockifier/tests/**'
Shouldn't this specify the two files currently in the dir instead? (So future files won't trigger this action)
Code quote:
'crates/blockifier/tests/**'
.github/workflows/blockifier_compiled_cairo.yml
line 58 at r11 (raw file):
with: repository: 'starkware-libs/cairo' fetch-depth: 0
By the docs
# Whether to fetch tags, even if fetch-depth > 0.
# Default: false
fetch-tags: ''
we only need the tags don't we?
Suggestion:
fetch-depth: 1
crates/blockifier/tests/feature_contracts_compatibility_test.rs
line 23 at r11 (raw file):
// Then, run the FIX_COMMAND above. // To test Cairo1 feature contracts, first clone the Cairo repo and checkout the required tag.
Suggestion:
To fix Cairo1
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, 4 unresolved discussions (waiting on @aner-starkware, @AvivYossef-starkware, @meship-starkware, @noaov1, and @TzahiTaub)
.github/workflows/blockifier_compiled_cairo.yml
line 14 at r11 (raw file):
Previously, TzahiTaub (Tzahi) wrote…
Shouldn't this specify the two files currently in the dir instead? (So future files won't trigger this action)
Done.
.github/workflows/blockifier_compiled_cairo.yml
line 58 at r11 (raw file):
Previously, TzahiTaub (Tzahi) wrote…
By the docs
# Whether to fetch tags, even if fetch-depth > 0. # Default: false fetch-tags: ''
we only need the tags don't we?
Done. thanks!
crates/blockifier/tests/feature_contracts_compatibility_test.rs
line 23 at r11 (raw file):
// Then, run the FIX_COMMAND above. // To test Cairo1 feature contracts, first clone the Cairo repo and checkout the required tag.
Done.
9b0ad2b
to
1c94532
Compare
4993d6c
to
feb2d27
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 r12, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @aner-starkware, @AvivYossef-starkware, @meship-starkware, and @noaov1)
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, 1 unresolved discussion (waiting on @aner-starkware, @AvivYossef-starkware, @meship-starkware, and @noaov1)
.github/workflows/blockifier_compiled_cairo.yml
line 58 at r11 (raw file):
Previously, dorimedini-starkware wrote…
Done. thanks!
didn't work, reverting
feb2d27
to
f8f23d8
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 1 files at r13.
Reviewable status: all files reviewed (commit messages unreviewed), 1 unresolved discussion (waiting on @aner-starkware, @AvivYossef-starkware, @meship-starkware, and @noaov1)
.github/workflows/blockifier_compiled_cairo.yml
line 58 at r11 (raw file):
Previously, dorimedini-starkware wrote…
didn't work, reverting
OK, pretty annoying. Do you wanna try something else or give up?
We can try:
- uses: actions/checkout@v4
with:
fetch-depth: 0
filter: object:type=tag
(reached to here from the above link)
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 all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @aner-starkware, @AvivYossef-starkware, @meship-starkware, and @noaov1)
f8f23d8
to
2bae019
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 3 files reviewed, 1 unresolved discussion (waiting on @aner-starkware, @AvivYossef-starkware, @meship-starkware, @noaov1, and @TzahiTaub)
.github/workflows/blockifier_compiled_cairo.yml
line 58 at r11 (raw file):
Previously, TzahiTaub (Tzahi) wrote…
OK, pretty annoying. Do you wanna try something else or give up?
We can try:- uses: actions/checkout@v4 with: fetch-depth: 0 filter: object:type=tag
(reached to here from the above link)
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 3 files reviewed, 1 unresolved discussion (waiting on @aner-starkware, @AvivYossef-starkware, @meship-starkware, @noaov1, and @TzahiTaub)
.github/workflows/blockifier_compiled_cairo.yml
line 58 at r11 (raw file):
Previously, dorimedini-starkware wrote…
nope
filter 'object:type' not supported
2bae019
to
d7ec00a
Compare
1c94532
to
eb66da4
Compare
d7ec00a
to
19b3812
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 all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @aner-starkware, @AvivYossef-starkware, @meship-starkware, and @noaov1)
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.
Dismissed @aner-starkware from a discussion.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @aner-starkware, @AvivYossef-starkware, @meship-starkware, and @noaov1)
This change is