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(blockifier): use infra_utils for fetching manifest dir #2289

Merged
merged 2 commits into from
Dec 8, 2024
Merged

chore(blockifier): use infra_utils for fetching manifest dir #2289

merged 2 commits into from
Dec 8, 2024

Conversation

dorimedini-starkware
Copy link
Collaborator

No description provided.

@dorimedini-starkware dorimedini-starkware self-assigned this Nov 26, 2024
@reviewable-StarkWare
Copy link

This change is Reviewable

@github-actions GitHub Actions
Copy link

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

Artifacts upload workflows:

@dorimedini-starkware dorimedini-starkware force-pushed the dori/use-infra-utils-manifest-dir branch from 5801bc5 to 5872835 Compare November 26, 2024 14:27
Itay-Tsabary-Starkware
Itay-Tsabary-Starkware approved these changes Nov 26, 2024
View reviewed changes
Copy link
Contributor

@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.

:lgtm:

Reviewed 7 of 7 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @dorimedini-starkware)

@dorimedini-starkware dorimedini-starkware force-pushed the dori/use-infra-utils-manifest-dir branch from 5872835 to 90d5809 Compare November 26, 2024 15:01
dorimedini-starkware
dorimedini-starkware commented Nov 26, 2024
View reviewed changes
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: 6 of 10 files reviewed, 1 unresolved discussion (waiting on @Itay-Tsabary-Starkware)

a discussion (no related file):
py side

@github-actions GitHub Actions
Copy link

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [34.492 ms 34.606 ms 34.755 ms]
change: [-4.4121% -2.6872% -1.1737%] (p = 0.00 < 0.05)
Performance has improved.
Found 11 outliers among 100 measurements (11.00%)
1 (1.00%) low mild
4 (4.00%) high mild
6 (6.00%) high severe

@dorimedini-starkware dorimedini-starkware force-pushed the dori/use-infra-utils-manifest-dir branch 2 times, most recently from d1f25bf to dad665e Compare November 27, 2024 10:50
@github-actions GitHub Actions
Copy link

Benchmark movements:
tree_computation_flow performance regressed!
tree_computation_flow time: [35.547 ms 36.090 ms 36.714 ms]
change: [+1.5727% +3.2200% +5.0517%] (p = 0.00 < 0.05)
Performance has regressed.
Found 14 outliers among 100 measurements (14.00%)
1 (1.00%) high mild
13 (13.00%) high severe

@codecov Codecov
Copy link

codecov bot commented Nov 27, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 42.85714% with 4 lines in your changes missing coverage. Please review.

Project coverage is 77.45%. Comparing base (e3165c4) to head (b5ce23f).
Report is 735 commits behind head on main.

Files with missing lines Patch % Lines
crates/infra_utils/src/path.rs 0.00% 3 Missing ⚠️
crates/blockifier/src/test_utils/cairo_compile.rs 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##             main    #2289       +/-   ##
===========================================
+ Coverage   40.10%   77.45%   +37.34%     
===========================================
  Files          26      386      +360     
  Lines        1895    41280    +39385     
  Branches     1895    41280    +39385     
===========================================
+ Hits          760    31972    +31212     
- Misses       1100     7042     +5942     
- Partials       35     2266     +2231

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

@dorimedini-starkware dorimedini-starkware force-pushed the dori/use-infra-utils-manifest-dir branch 3 times, most recently from 3c91bb9 to fa7cde1 Compare December 4, 2024 11:50
meship-starkware
meship-starkware approved these changes Dec 4, 2024
View reviewed changes
Copy link
Contributor

@meship-starkware meship-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 4 files at r2, 3 of 10 files at r5, 7 of 7 files at r6, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware)

@dorimedini-starkware dorimedini-starkware force-pushed the dori/use-infra-utils-manifest-dir branch from fa7cde1 to fede8cb Compare December 5, 2024 13:27
dorimedini-starkware
dorimedini-starkware commented Dec 5, 2024
View reviewed changes
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: :shipit: complete! all files reviewed, all discussions resolved (waiting on @dorimedini-starkware)

a discussion (no related file):

Previously, dorimedini-starkware wrote…

py side

passed

meship-starkware
meship-starkware approved these changes Dec 5, 2024
View reviewed changes
Copy link
Contributor

@meship-starkware meship-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 7 files at r7, 8 of 8 files at r8, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @dorimedini-starkware)

@dorimedini-starkware dorimedini-starkware force-pushed the dori/use-infra-utils-manifest-dir branch from fede8cb to 345dc29 Compare December 5, 2024 15:09
meship-starkware
meship-starkware approved these changes Dec 5, 2024
View reviewed changes
Copy link
Contributor

@meship-starkware meship-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 8 of 8 files at r10, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @dorimedini-starkware)

@dorimedini-starkware dorimedini-starkware force-pushed the dori/use-infra-utils-manifest-dir branch from 345dc29 to b5ce23f Compare December 8, 2024 08:26
meship-starkware
meship-starkware approved these changes Dec 8, 2024
View reviewed changes
Copy link
Contributor

@meship-starkware meship-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 8 of 8 files at r12, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @dorimedini-starkware)

@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 8, 2024

Benchmark movements:
tree_computation_flow performance regressed!
tree_computation_flow time: [35.172 ms 35.409 ms 35.687 ms]
change: [+1.8988% +2.6334% +3.5114%] (p = 0.00 < 0.05)
Performance has regressed.
Found 18 outliers among 100 measurements (18.00%)
1 (1.00%) low mild
6 (6.00%) high mild
11 (11.00%) high severe

@dorimedini-starkware dorimedini-starkware merged commit a19cd98 into main Dec 8, 2024
23 checks passed
@dorimedini-starkware dorimedini-starkware deleted the dori/use-infra-utils-manifest-dir branch December 8, 2024 08:52
@github-actions github-actions bot locked and limited conversation to collaborators Dec 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@meship-starkware meship-starkware meship-starkware approved these changes

@Itay-Tsabary-Starkware Itay-Tsabary-Starkware Itay-Tsabary-Starkware approved these changes

Assignees

@dorimedini-starkware dorimedini-starkware

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dorimedini-starkware @reviewable-StarkWare @Itay-Tsabary-Starkware @meship-starkware