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.

Sign up for GitHub

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

Jump to bottom

chore(sequencer_node): replace run node shell command #2266

Merged
merged 1 commit into from
Nov 26, 2024
Merged

chore(sequencer_node): replace run node shell command #2266

merged 1 commit into from
Nov 26, 2024

Conversation

Itay-Tsabary-Starkware
Copy link
Contributor

@Itay-Tsabary-Starkware Itay-Tsabary-Starkware commented Nov 25, 2024 •
edited
Loading

No description provided.

This was referenced Nov 25, 2024
Closed
Merged
Closed
Merged
@reviewable-StarkWare
Copy link

This change is Reviewable

@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 25, 2024 •
edited
Loading

Artifacts upload workflows:

@github-actions GitHub Actions
Copy link

Benchmark movements:
full_committer_flow performance regressed!
full_committer_flow time: [30.176 ms 30.228 ms 30.289 ms]
change: [+1.5258% +1.8010% +2.0850%] (p = 0.00 < 0.05)
Performance has regressed.
Found 3 outliers among 100 measurements (3.00%)
2 (2.00%) high mild
1 (1.00%) high severe

@Itay-Tsabary-Starkware Itay-Tsabary-Starkware force-pushed the spr/main/3dd1cf11 branch 3 times, most recently from ae16c8c to 02a46d5 Compare November 25, 2024 10:37
@Itay-Tsabary-Starkware Itay-Tsabary-Starkware mentioned this pull request Nov 25, 2024
Open
@github-actions GitHub Actions
Copy link

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [34.462 ms 34.559 ms 34.678 ms]
change: [-4.0157% -2.4560% -1.0414%] (p = 0.00 < 0.05)
Performance has improved.
Found 9 outliers among 100 measurements (9.00%)
5 (5.00%) high mild
4 (4.00%) high severe

@Itay-Tsabary-Starkware Itay-Tsabary-Starkware force-pushed the spr/main/3dd1cf11 branch 3 times, most recently from 2da82f0 to 04ff5b3 Compare November 25, 2024 12:38
@Itay-Tsabary-Starkware Itay-Tsabary-Starkware mentioned this pull request Nov 25, 2024
Closed
@Itay-Tsabary-Starkware Itay-Tsabary-Starkware force-pushed the spr/main/3dd1cf11 branch 2 times, most recently from 9004529 to 3bb4e28 Compare November 25, 2024 13:57
ArniStarkware
ArniStarkware approved these changes Nov 25, 2024
View reviewed changes
Copy link
Contributor

@ArniStarkware ArniStarkware 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 r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Itay-Tsabary-Starkware)

@Itay-Tsabary-Starkware Itay-Tsabary-Starkware changed the base branch from spr/main/7f25ebe8 to main November 25, 2024 17:38
@Itay-Tsabary-Starkware Itay-Tsabary-Starkware force-pushed the spr/main/3dd1cf11 branch 2 times, most recently from 8e80971 to 000aaa8 Compare November 25, 2024 17:40
@codecov Codecov
Copy link

codecov bot commented Nov 25, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 33.33333% with 2 lines in your changes missing coverage. Please review.

Project coverage is 77.10%. Comparing base (e3165c4) to head (63d8744).
Report is 591 commits behind head on main.

Files with missing lines Patch % Lines
...rknet_sequencer_node/src/test_utils/compilation.rs 33.33% 2 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##             main    #2266       +/-   ##
===========================================
+ Coverage   40.10%   77.10%   +36.99%     
===========================================
  Files          26      386      +360     
  Lines        1895    40525    +38630     
  Branches     1895    40525    +38630     
===========================================
+ Hits          760    31245    +30485     
- Misses       1100     6977     +5877     
- Partials       35     2303     +2268

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ArniStarkware
Copy link
Contributor

crates/starknet_sequencer_node/src/test_utils/compilation.rs line 7 at r2 (raw file):

use tracing::info;

pub const NODE_EXECUTABLE_PATH: &str = "target/debug/starknet_sequencer_node";

This is always debug as this is a test-util, this will always be called in a debug context. Correct?

Code quote:

pub const NODE_EXECUTABLE_PATH: &str = "target/debug/starknet_sequencer_node";

ArniStarkware
ArniStarkware approved these changes Nov 26, 2024
View reviewed changes
Copy link
Contributor

@ArniStarkware ArniStarkware 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 3 of 3 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Itay-Tsabary-Starkware)

Itay-Tsabary-Starkware
Itay-Tsabary-Starkware commented Nov 26, 2024
View reviewed changes
Copy link
Contributor Author

@Itay-Tsabary-Starkware Itay-Tsabary-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 3 files at r1, 3 of 3 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Itay-Tsabary-Starkware)

Itay-Tsabary-Starkware
Itay-Tsabary-Starkware commented Nov 26, 2024
View reviewed changes
Copy link
Contributor Author

@Itay-Tsabary-Starkware Itay-Tsabary-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: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Itay-Tsabary-Starkware)

@github-actions GitHub Actions
Copy link

Benchmark movements:
tree_computation_flow performance regressed!
tree_computation_flow time: [34.120 ms 34.498 ms 34.929 ms]
change: [+1.6293% +2.7172% +3.9128%] (p = 0.00 < 0.05)
Performance has regressed.
Found 17 outliers among 100 measurements (17.00%)
3 (3.00%) high mild
14 (14.00%) high severe

ArniStarkware
ArniStarkware approved these changes Nov 26, 2024
View reviewed changes
Copy link
Contributor

@ArniStarkware ArniStarkware left a 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: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Itay-Tsabary-Starkware)

ArniStarkware
ArniStarkware approved these changes Nov 26, 2024
View reviewed changes
Copy link
Contributor

@ArniStarkware ArniStarkware 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.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Itay-Tsabary-Starkware)

ArniStarkware
ArniStarkware approved these changes Nov 26, 2024
View reviewed changes
Copy link
Contributor

@ArniStarkware ArniStarkware left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Itay-Tsabary-Starkware)

@Itay-Tsabary-Starkware Itay-Tsabary-Starkware merged commit 9617c00 into main Nov 26, 2024
20 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Nov 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ArniStarkware ArniStarkware ArniStarkware approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Itay-Tsabary-Starkware @reviewable-StarkWare @ArniStarkware