Skip to content

Conversation

@dorimedini-starkware
Copy link
Collaborator

No description provided.

@reviewable-StarkWare
Copy link

This change is Reviewable

Copy link
Collaborator Author

dorimedini-starkware commented May 1, 2025

@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-test_apollo_starknet_os_program_os_constants_test branch from 1b132cd to 1e3cfe3 Compare May 1, 2025 14:38
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-feat_apollo_starknet_os_program_add_fixer_for_os_constants_test branch from 021abe4 to ecd2e80 Compare May 1, 2025 14:38
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-test_apollo_starknet_os_program_os_constants_test branch from 1e3cfe3 to 5db3b08 Compare May 1, 2025 14:43
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-feat_apollo_starknet_os_program_add_fixer_for_os_constants_test branch from ecd2e80 to c4aae1f Compare May 1, 2025 14:43
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-test_apollo_starknet_os_program_os_constants_test branch from 5db3b08 to a5c63b3 Compare May 1, 2025 14:50
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-feat_apollo_starknet_os_program_add_fixer_for_os_constants_test branch from c4aae1f to 1959629 Compare May 1, 2025 14:50
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-test_apollo_starknet_os_program_os_constants_test branch from a5c63b3 to 84d3238 Compare May 2, 2025 13:59
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-feat_apollo_starknet_os_program_add_fixer_for_os_constants_test branch from 1959629 to 62577cc Compare May 2, 2025 13:59
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-test_apollo_starknet_os_program_os_constants_test branch from 84d3238 to 122bde7 Compare May 2, 2025 14:06
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-feat_apollo_starknet_os_program_add_fixer_for_os_constants_test branch from 62577cc to 40e726c Compare May 2, 2025 14:06
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-test_apollo_starknet_os_program_os_constants_test branch from 122bde7 to 53ef04c Compare May 2, 2025 14:13
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-feat_apollo_starknet_os_program_add_fixer_for_os_constants_test branch from 40e726c to 2b5aea8 Compare May 2, 2025 14:13
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-test_apollo_starknet_os_program_os_constants_test branch from 53ef04c to b367524 Compare May 2, 2025 14:19
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-feat_apollo_starknet_os_program_add_fixer_for_os_constants_test branch from 2b5aea8 to adbd99c Compare May 2, 2025 14:19
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-test_apollo_starknet_os_program_os_constants_test branch from b367524 to b4f3245 Compare May 5, 2025 11:01
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-feat_apollo_starknet_os_program_add_fixer_for_os_constants_test branch from adbd99c to a7774da Compare May 5, 2025 11:01
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-test_apollo_starknet_os_program_os_constants_test branch from b4f3245 to 682814a Compare May 5, 2025 12:59
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-feat_apollo_starknet_os_program_add_fixer_for_os_constants_test branch from a7774da to 7739e77 Compare May 5, 2025 12:59
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-test_apollo_starknet_os_program_os_constants_test branch from 682814a to 3d41b01 Compare May 5, 2025 13:10
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-feat_apollo_starknet_os_program_add_fixer_for_os_constants_test branch from 185915b to 086fe71 Compare May 25, 2025 17:12
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-test_apollo_starknet_os_program_os_constants_test branch from bbcd655 to b3e6c02 Compare May 26, 2025 08:04
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-feat_apollo_starknet_os_program_add_fixer_for_os_constants_test branch from 086fe71 to 7e010c8 Compare May 26, 2025 08:04
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-test_apollo_starknet_os_program_os_constants_test branch from b3e6c02 to 9b8306a Compare May 26, 2025 09:53
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-feat_apollo_starknet_os_program_add_fixer_for_os_constants_test branch from 7e010c8 to c64cd8f Compare May 26, 2025 09:53
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-test_apollo_starknet_os_program_os_constants_test branch from 9b8306a to a337f63 Compare May 26, 2025 11:55
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-feat_apollo_starknet_os_program_add_fixer_for_os_constants_test branch from c64cd8f to 57504a2 Compare May 26, 2025 11:55
@dorimedini-starkware dorimedini-starkware marked this pull request as ready for review May 26, 2025 14:19
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-test_apollo_starknet_os_program_os_constants_test branch from a337f63 to 4a2fb35 Compare May 26, 2025 14:50
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-feat_apollo_starknet_os_program_add_fixer_for_os_constants_test branch from 57504a2 to 77af9e1 Compare May 26, 2025 14:50
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-test_apollo_starknet_os_program_os_constants_test branch from 4a2fb35 to caac33d Compare May 27, 2025 10:29
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-feat_apollo_starknet_os_program_add_fixer_for_os_constants_test branch from 77af9e1 to bd987f8 Compare May 27, 2025 10:29
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-test_apollo_starknet_os_program_os_constants_test branch from caac33d to 854e41a Compare May 27, 2025 10:32
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-feat_apollo_starknet_os_program_add_fixer_for_os_constants_test branch from bd987f8 to 2acf100 Compare May 27, 2025 10:32
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-test_apollo_starknet_os_program_os_constants_test branch from 854e41a to 7b14bab Compare May 28, 2025 15:04
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-feat_apollo_starknet_os_program_add_fixer_for_os_constants_test branch from 2acf100 to d1520cd Compare May 28, 2025 15:04
Copy link
Contributor

@avivg-starkware avivg-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 r3, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @AvivYossef-starkware)


crates/apollo_starknet_os_program/src/constants_test.rs line 215 at r3 (raw file):

/// ```bash
/// FIX_OS_CONSTANTS=1 cargo test -p apollo_starknet_os_program test_os_constants
/// ```

Perhaps more explicit?

Suggestion:

/// Test that `constants.cairo` generated from OS versioned constants, matches the existing file.
/// To fix this test, run:
/// ```bash
/// FIX_OS_CONSTANTS=1 cargo test -p apollo_starknet_os_program test_os_constants
/// ```

crates/apollo_starknet_os_program/src/constants_test.rs line 219 at r3 (raw file):

fn test_os_constants() {
    verify_cairo0_compiler_deps();
    let generated = generate_constants_file();

Also, for extra clarity
(not sure if needed, perhaps the function's doc might be sufficient)

My main goal is to make it visible in the function which 'source' file drives which 'output' file.

Anyway, non-blocking, as you see fit, is great.

Suggestion:

// generate `constants.cairo` from the current OS constants.
let generated = generate_constants_file();

@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-test_apollo_starknet_os_program_os_constants_test branch from 7b14bab to 0f85e49 Compare May 29, 2025 14:02
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-feat_apollo_starknet_os_program_add_fixer_for_os_constants_test branch 2 times, most recently from c1aae23 to b87f63b Compare May 29, 2025 14:59
Copy link
Contributor

@avivg-starkware avivg-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 r4, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @AvivYossef-starkware)

@dorimedini-starkware dorimedini-starkware changed the base branch from 05-01-test_apollo_starknet_os_program_os_constants_test to main-v0.14.0 May 30, 2025 09:36
@dorimedini-starkware dorimedini-starkware force-pushed the 05-01-feat_apollo_starknet_os_program_add_fixer_for_os_constants_test branch from b87f63b to a420f52 Compare May 30, 2025 09:36
@graphite-app
Copy link

graphite-app bot commented May 30, 2025

Merge activity

  • May 30, 9:36 AM UTC: Graphite rebased this pull request, because this pull request is set to merge when ready.

@dorimedini-starkware dorimedini-starkware added this pull request to the merge queue May 30, 2025
Merged via the queue into main-v0.14.0 with commit af1bae1 May 30, 2025
11 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jun 1, 2025
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.

5 participants