Skip to content

refactor(starknet_gateway): remove unused gateway compiler#3499

Merged
noamsp-starkware merged 8 commits intonoam.s/feat_starknet_gateway_get_executable_tx_from_class_managerfrom
noam.s/refactor_starknet_gateway_remove_unused_gateway_compiler
Jan 22, 2025
Merged

refactor(starknet_gateway): remove unused gateway compiler#3499
noamsp-starkware merged 8 commits intonoam.s/feat_starknet_gateway_get_executable_tx_from_class_managerfrom
noam.s/refactor_starknet_gateway_remove_unused_gateway_compiler

Conversation

@noamsp-starkware
Copy link
Contributor

@noamsp-starkware noamsp-starkware commented Jan 20, 2025

No description provided.

This was referenced Jan 20, 2025
Merged
Merged
Closed
@reviewable-StarkWare
Copy link

reviewable-StarkWare commented Jan 20, 2025

This change is Reviewable

@noamsp-starkware noamsp-starkware mentioned this pull request Jan 20, 2025
Merged
@noamsp-starkware Graphite App
Copy link
Contributor Author

noamsp-starkware commented Jan 20, 2025 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

This was referenced Jan 20, 2025
Merged
Merged
Merged
@noamsp-starkware noamsp-starkware marked this pull request as ready for review January 20, 2025 09:36
This was referenced Jan 20, 2025
Merged
Merged
Merged
Merged
@noamsp-starkware noamsp-starkware force-pushed the noam.s/feat_starknet_gateway_get_executable_tx_from_class_manager branch from cb67b17 to 26ec928 Compare January 20, 2025 14:18
@noamsp-starkware noamsp-starkware force-pushed the noam.s/refactor_starknet_gateway_remove_unused_gateway_compiler branch from 5486b68 to 2f6cf19 Compare January 20, 2025 14:19
ShahakShama
ShahakShama approved these changes Jan 20, 2025
View reviewed changes
Copy link
Collaborator

@ShahakShama ShahakShama 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 6 of 6 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @noamsp-starkware)

@noamsp-starkware noamsp-starkware force-pushed the noam.s/feat_starknet_gateway_get_executable_tx_from_class_manager branch from 26ec928 to b4dbd5f Compare January 21, 2025 05:34
@noamsp-starkware noamsp-starkware force-pushed the noam.s/refactor_starknet_gateway_remove_unused_gateway_compiler branch from 2f6cf19 to 3786f9e Compare January 21, 2025 05:34
alonh5
alonh5 suggested changes Jan 21, 2025
View reviewed changes
Copy link
Contributor

@alonh5 alonh5 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: 3 of 6 files reviewed, 1 unresolved discussion (waiting on @noamsp-starkware and @ShahakShama)

crates/starknet_gateway/src/gateway.rs line 105 at r2 (raw file):

    // TODO(noamsp): remove gatewayCompiler from here and erase it
    #[allow(dead_code)]
    gateway_compiler: Arc<GatewayCompiler>,

Can you also erase it?

@noamsp-starkware noamsp-starkware force-pushed the noam.s/feat_starknet_gateway_get_executable_tx_from_class_manager branch from b4dbd5f to d111096 Compare January 21, 2025 12:52
@noamsp-starkware noamsp-starkware force-pushed the noam.s/refactor_starknet_gateway_remove_unused_gateway_compiler branch from 3786f9e to 24a9f6a Compare January 21, 2025 12:52
noamsp-starkware
noamsp-starkware commented Jan 21, 2025
View reviewed changes
Copy link
Contributor Author

@noamsp-starkware noamsp-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: 2 of 8 files reviewed, 1 unresolved discussion (waiting on @alonh5 and @ShahakShama)

crates/starknet_gateway/src/gateway.rs line 105 at r2 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

Can you also erase it?

Done.

alonh5
alonh5 approved these changes Jan 21, 2025
View reviewed changes
Copy link
Contributor

@alonh5 alonh5 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 6 files at r1, 1 of 3 files at r2, 5 of 5 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @noamsp-starkware)

@noamsp-starkware noamsp-starkware force-pushed the noam.s/feat_starknet_gateway_get_executable_tx_from_class_manager branch from d111096 to 8796161 Compare January 21, 2025 15:33
@noamsp-starkware noamsp-starkware force-pushed the noam.s/refactor_starknet_gateway_remove_unused_gateway_compiler branch from 24a9f6a to 352a55a Compare January 21, 2025 15:33
@noamsp-starkware noamsp-starkware mentioned this pull request Jan 21, 2025
Merged
alonh5
alonh5 approved these changes Jan 21, 2025
View reviewed changes
Copy link
Contributor

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

@noamsp-starkware noamsp-starkware mentioned this pull request Jan 21, 2025
Closed
@github-actions
Copy link

github-actions bot commented Jan 22, 2025

Artifacts upload workflows:

@noamsp-starkware noamsp-starkware merged commit fc64464 into noam.s/feat_starknet_gateway_get_executable_tx_from_class_manager Jan 22, 2025
14 of 18 checks passed
@github-actions
Copy link

github-actions bot commented Jan 22, 2025

Benchmark movements:
tree_computation_flow performance regressed!
tree_computation_flow time: [35.908 ms 36.383 ms 36.943 ms]
change: [+3.2114% +4.5120% +6.4273%] (p = 0.00 < 0.05)
Performance has regressed.
Found 13 outliers among 100 measurements (13.00%)
2 (2.00%) high mild
11 (11.00%) high severe

@github-actions github-actions bot locked and limited conversation to collaborators Jan 25, 2025
@noamsp-starkware noamsp-starkware deleted the noam.s/refactor_starknet_gateway_remove_unused_gateway_compiler branch January 27, 2025 11:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@ShahakShama ShahakShama ShahakShama approved these changes

+1 more reviewer

@alonh5 alonh5 alonh5 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@noamsp-starkware @reviewable-StarkWare @ShahakShama @alonh5