Skip to content

Conversation

@Idanh-starkware
Copy link
Contributor

@Idanh-starkware Idanh-starkware commented Aug 21, 2025

Type

  • feature
  • bugfix
  • dev (no functional changes, no API changes)
  • fmt (formatting, renaming)
  • build
  • docs
  • testing

Description

Breaking changes?

  • yes
  • no

This change is Reviewable

Copy link
Contributor Author

Idanh-starkware commented Aug 21, 2025

@Idanh-starkware Idanh-starkware mentioned this pull request Aug 21, 2025
9 tasks
@Idanh-starkware Idanh-starkware force-pushed the idanh/adding_simple_bootloader_hints_tests branch from 1c093d9 to 3498363 Compare August 21, 2025 14:38
@Idanh-starkware Idanh-starkware force-pushed the idanh/enableing_execute_task_tests branch from 722e685 to 1a731f2 Compare August 21, 2025 14:38
@Idanh-starkware Idanh-starkware changed the base branch from idanh/enableing_execute_task_tests to graphite-base/208 August 27, 2025 15:50
@Idanh-starkware Idanh-starkware force-pushed the idanh/adding_simple_bootloader_hints_tests branch from 3498363 to 0a402b5 Compare August 27, 2025 15:50
@Idanh-starkware Idanh-starkware changed the base branch from graphite-base/208 to main August 27, 2025 15:50
@Idanh-starkware Idanh-starkware changed the base branch from main to graphite-base/208 August 28, 2025 06:54
@Idanh-starkware Idanh-starkware force-pushed the idanh/adding_simple_bootloader_hints_tests branch from 0a402b5 to c2199fe Compare August 28, 2025 06:54
@Idanh-starkware Idanh-starkware changed the base branch from graphite-base/208 to idanh/bump_starknet_crypto_to_0.7.4 August 28, 2025 06:54
@Idanh-starkware Idanh-starkware self-assigned this Aug 28, 2025
@Idanh-starkware Idanh-starkware marked this pull request as ready for review August 28, 2025 06:56
@Idanh-starkware Idanh-starkware force-pushed the idanh/adding_simple_bootloader_hints_tests branch from c2199fe to d9cf347 Compare August 28, 2025 07:10
Copy link
Contributor

@OmriEshhar1 OmriEshhar1 left a comment

Choose a reason for hiding this comment

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

@OmriEshhar1 reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @YairVaknin-starkware)


crates/cairo-program-runner-lib/src/hints/simple_bootloader_hints.rs line 432 at r1 (raw file):

    #[fixture]
    fn fibonacci() -> Program {

this fixture is used in multiple tests.
can't it be put in a common place where it can be used by all tests (without code duplication)?


crates/cairo-program-runner-lib/src/hints/simple_bootloader_hints.rs line 684 at r1 (raw file):

    /// This function prepares a VirtualMachine instance for testing simulate_builtin hints.
    fn prepare_vm_for_simulate_builtin(

maybe put all the auxiliary function at the beginning of the module

@Idanh-starkware Idanh-starkware force-pushed the idanh/adding_simple_bootloader_hints_tests branch from d9cf347 to 1c188b9 Compare September 3, 2025 10:54
@Idanh-starkware Idanh-starkware force-pushed the idanh/bump_starknet_crypto_to_0.7.4 branch from 590a91a to 2f27437 Compare September 3, 2025 10:54
@Idanh-starkware Idanh-starkware force-pushed the idanh/adding_simple_bootloader_hints_tests branch from 1c188b9 to db0770e Compare September 4, 2025 12:11
@Idanh-starkware Idanh-starkware force-pushed the idanh/bump_starknet_crypto_to_0.7.4 branch from 5c054f6 to b4788b8 Compare September 4, 2025 12:11
Copy link
Contributor Author

@Idanh-starkware Idanh-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 4 files reviewed, 2 unresolved discussions (waiting on @OmriEshhar1 and @YairVaknin-starkware)


crates/cairo-program-runner-lib/src/hints/simple_bootloader_hints.rs line 432 at r1 (raw file):

Previously, OmriEshhar1 wrote…

this fixture is used in multiple tests.
can't it be put in a common place where it can be used by all tests (without code duplication)?

Done.


crates/cairo-program-runner-lib/src/hints/simple_bootloader_hints.rs line 684 at r1 (raw file):

Previously, OmriEshhar1 wrote…

maybe put all the auxiliary function at the beginning of the module

Done.

Copy link
Contributor

@OmriEshhar1 OmriEshhar1 left a comment

Choose a reason for hiding this comment

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

:lgtm:

@OmriEshhar1 reviewed 4 of 4 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @YairVaknin-starkware)

@Idanh-starkware Idanh-starkware changed the base branch from idanh/bump_starknet_crypto_to_0.7.4 to main September 7, 2025 12:46
@Idanh-starkware Idanh-starkware enabled auto-merge (squash) September 7, 2025 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants