Skip to content
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(blockifier): add text file with current compiler version for CI use #195

Conversation

dorimedini-starkware
Copy link
Collaborator

@dorimedini-starkware dorimedini-starkware commented Jul 30, 2024

This change is Reviewable

@dorimedini-starkware dorimedini-starkware self-assigned this Jul 30, 2024
@dorimedini-starkware dorimedini-starkware force-pushed the 07-30-chore_blockifier_verify_local_cairo_repo_state_before_compiling branch from e3b2330 to e62e313 Compare July 30, 2024 13:21
@dorimedini-starkware dorimedini-starkware force-pushed the 07-30-chore_blockifier_add_text_file_with_current_compiler_version_for_ci_use branch from 2618e0f to e073a68 Compare July 30, 2024 13:21
@dorimedini-starkware dorimedini-starkware force-pushed the 07-30-chore_blockifier_verify_local_cairo_repo_state_before_compiling branch from e62e313 to 4fe6734 Compare July 31, 2024 16:34
@dorimedini-starkware dorimedini-starkware force-pushed the 07-30-chore_blockifier_add_text_file_with_current_compiler_version_for_ci_use branch from e073a68 to 0cc6a4f Compare July 31, 2024 16:34
@dorimedini-starkware dorimedini-starkware force-pushed the 07-30-chore_blockifier_verify_local_cairo_repo_state_before_compiling branch from 4fe6734 to 9facef4 Compare July 31, 2024 16:42
@dorimedini-starkware dorimedini-starkware force-pushed the 07-30-chore_blockifier_add_text_file_with_current_compiler_version_for_ci_use branch from 0cc6a4f to f400d64 Compare July 31, 2024 16:42
@dorimedini-starkware dorimedini-starkware force-pushed the 07-30-chore_blockifier_verify_local_cairo_repo_state_before_compiling branch from 9facef4 to 0d099c5 Compare July 31, 2024 16:52
@dorimedini-starkware dorimedini-starkware force-pushed the 07-30-chore_blockifier_add_text_file_with_current_compiler_version_for_ci_use branch from f400d64 to 58aca6d Compare July 31, 2024 16:52
@dorimedini-starkware dorimedini-starkware force-pushed the 07-30-chore_blockifier_verify_local_cairo_repo_state_before_compiling branch from 0d099c5 to a12e845 Compare July 31, 2024 16:54
@dorimedini-starkware dorimedini-starkware force-pushed the 07-30-chore_blockifier_add_text_file_with_current_compiler_version_for_ci_use branch from 58aca6d to 7b79e6f Compare July 31, 2024 16:54
@aner-starkware
Copy link
Contributor

crates/blockifier/tests/cairo1_compiler_tag.txt line 1 at r2 (raw file):

v2.7.0-rc.3

imo, this is not a good idea. Apart from having to maintain 2 locations now instead of only the .toml, this also creates a "second point of truth", which is what you're trying to avoid.

Code quote:

v2.7.0-rc.3

Copy link
Contributor

@aner-starkware aner-starkware left a 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.
Reviewable status: 0 of 5 files reviewed, 2 unresolved discussions (waiting on @AvivYossef-starkware, @dorimedini-starkware, @meship-starkware, and @noaov1)


crates/blockifier/tests/legacy_cairo1_compiler_tag.txt line 1 at r2 (raw file):

v2.1.0

What's the idea here?

Code quote:

v2.1.0

@dorimedini-starkware dorimedini-starkware force-pushed the 07-30-chore_blockifier_verify_local_cairo_repo_state_before_compiling branch from a12e845 to d2dd1b8 Compare August 1, 2024 08:50
@dorimedini-starkware dorimedini-starkware force-pushed the 07-30-chore_blockifier_add_text_file_with_current_compiler_version_for_ci_use branch from 7b79e6f to 738d7f2 Compare August 1, 2024 08:50
@dorimedini-starkware dorimedini-starkware force-pushed the 07-30-chore_blockifier_verify_local_cairo_repo_state_before_compiling branch from d2dd1b8 to 10d4789 Compare August 1, 2024 08:53
@dorimedini-starkware dorimedini-starkware force-pushed the 07-30-chore_blockifier_add_text_file_with_current_compiler_version_for_ci_use branch from 8ad0c34 to 1f6f2aa Compare August 5, 2024 13:00
Copy link
Collaborator Author

@dorimedini-starkware dorimedini-starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 5 files reviewed, 1 unresolved discussion (waiting on @aner-starkware, @AvivYossef-starkware, @meship-starkware, and @noaov1)


crates/blockifier/tests/legacy_cairo1_compiler_tag.txt line 1 at r2 (raw file):

Previously, aner-starkware wrote…

Why not hardcode in the CI instead of in a different file? The test can check that the hardcoded constant in the CI is equal to the expected.

actually I think it's not even needed anymore, see top of the stack

Copy link
Contributor

@aner-starkware aner-starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 5 files reviewed, all discussions resolved (waiting on @AvivYossef-starkware, @meship-starkware, and @noaov1)

@dorimedini-starkware dorimedini-starkware force-pushed the 07-30-chore_blockifier_verify_local_cairo_repo_state_before_compiling branch from 031e4da to 6e5f836 Compare August 6, 2024 12:16
@dorimedini-starkware dorimedini-starkware force-pushed the 07-30-chore_blockifier_add_text_file_with_current_compiler_version_for_ci_use branch from 1f6f2aa to e11a98a Compare August 6, 2024 12:16
@dorimedini-starkware dorimedini-starkware force-pushed the 07-30-chore_blockifier_verify_local_cairo_repo_state_before_compiling branch from 6e5f836 to 9942ae2 Compare August 6, 2024 13:30
@dorimedini-starkware dorimedini-starkware force-pushed the 07-30-chore_blockifier_add_text_file_with_current_compiler_version_for_ci_use branch 2 times, most recently from 4b6aa3d to ae71e03 Compare August 7, 2024 07:00
@dorimedini-starkware dorimedini-starkware force-pushed the 07-30-chore_blockifier_verify_local_cairo_repo_state_before_compiling branch from 9942ae2 to 0895bc5 Compare August 7, 2024 14:51
@dorimedini-starkware dorimedini-starkware force-pushed the 07-30-chore_blockifier_add_text_file_with_current_compiler_version_for_ci_use branch from ae71e03 to 3fd07d1 Compare August 7, 2024 14:52
@dorimedini-starkware dorimedini-starkware force-pushed the 07-30-chore_blockifier_verify_local_cairo_repo_state_before_compiling branch from 0895bc5 to 388820c Compare August 7, 2024 16:16
@dorimedini-starkware dorimedini-starkware force-pushed the 07-30-chore_blockifier_add_text_file_with_current_compiler_version_for_ci_use branch from 3fd07d1 to 07ec736 Compare August 7, 2024 16:17
@dorimedini-starkware dorimedini-starkware marked this pull request as ready for review August 8, 2024 09:51
@dorimedini-starkware dorimedini-starkware force-pushed the 07-30-chore_blockifier_verify_local_cairo_repo_state_before_compiling branch from 388820c to 7a76151 Compare August 11, 2024 07:45
@dorimedini-starkware dorimedini-starkware force-pushed the 07-30-chore_blockifier_add_text_file_with_current_compiler_version_for_ci_use branch from 07ec736 to f623af9 Compare August 11, 2024 07:45
@dorimedini-starkware dorimedini-starkware force-pushed the 07-30-chore_blockifier_verify_local_cairo_repo_state_before_compiling branch from 7a76151 to 83dc987 Compare August 11, 2024 08:55
@dorimedini-starkware dorimedini-starkware force-pushed the 07-30-chore_blockifier_add_text_file_with_current_compiler_version_for_ci_use branch from f623af9 to 1b9c871 Compare August 11, 2024 08:55
@dorimedini-starkware dorimedini-starkware force-pushed the 07-30-chore_blockifier_verify_local_cairo_repo_state_before_compiling branch from 83dc987 to b29a56b Compare August 11, 2024 09:35
@dorimedini-starkware dorimedini-starkware force-pushed the 07-30-chore_blockifier_add_text_file_with_current_compiler_version_for_ci_use branch from 1b9c871 to 452fc59 Compare August 11, 2024 09:35
Copy link
Contributor

@TzahiTaub TzahiTaub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 4 files at r6, 2 of 4 files at r7, 2 of 2 files at r9, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @AvivYossef-starkware, @meship-starkware, and @noaov1)

@dorimedini-starkware dorimedini-starkware force-pushed the 07-30-chore_blockifier_verify_local_cairo_repo_state_before_compiling branch from b29a56b to 9d6f7ff Compare August 11, 2024 12:58
@dorimedini-starkware dorimedini-starkware force-pushed the 07-30-chore_blockifier_add_text_file_with_current_compiler_version_for_ci_use branch from 452fc59 to ce6fbbf Compare August 11, 2024 12:58
Copy link
Contributor

@TzahiTaub TzahiTaub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 2 of 2 files at r10, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @AvivYossef-starkware, @meship-starkware, and @noaov1)

Copy link
Collaborator Author

dorimedini-starkware commented Aug 11, 2024

Merge activity

@dorimedini-starkware dorimedini-starkware changed the base branch from 07-30-chore_blockifier_verify_local_cairo_repo_state_before_compiling to graphite-base/195 August 11, 2024 16:24
@dorimedini-starkware dorimedini-starkware changed the base branch from graphite-base/195 to main August 11, 2024 16:32
@dorimedini-starkware dorimedini-starkware force-pushed the 07-30-chore_blockifier_add_text_file_with_current_compiler_version_for_ci_use branch from ce6fbbf to 688beb1 Compare August 11, 2024 16:33
@dorimedini-starkware dorimedini-starkware merged commit 10b4be1 into main Aug 11, 2024
14 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Aug 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants