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

[nativeaot][tests] Update bundle zip path for NativeAOT sample app #87049

Merged
merged 15 commits into from
Jun 5, 2023
Merged

[nativeaot][tests] Update bundle zip path for NativeAOT sample app #87049

merged 15 commits into from
Jun 5, 2023

Conversation

kotlarmilos
Copy link
Member

@kotlarmilos kotlarmilos commented Jun 2, 2023 •
edited
Loading

This PR updates bundle zip path for NativeAOT sample app by using a shared job between Mono and Native AOT measurements.

Follow-up #85317

@kotlarmilos kotlarmilos added this to the 8.0.0 milestone Jun 2, 2023
@kotlarmilos kotlarmilos self-assigned this Jun 2, 2023
@ghost
Copy link

ghost commented Jun 2, 2023

Tagging subscribers to this area: @directhex
See info in area-owners.md if you want to be subscribed.

Issue Details

This PR updates bundle zip path for NativeAOT sample app.

Author: kotlarmilos
Assignees: kotlarmilos
Labels:

area-Infrastructure-mono
Milestone: 8.0.0

@kotlarmilos kotlarmilos requested a review from radical as a code owner June 2, 2023 21:51
@build-analysis build-analysis bot mentioned this pull request Jun 3, 2023
Closed
@kotlarmilos kotlarmilos requested a review from ivanpovazan June 5, 2023 11:36
@kotlarmilos
Copy link
Member Author

This PR is ready for review. The performance results can be found at the dotnet-runtime-perf pipeline. Please note that there is a known issue dotnet/performance#3054 with the Mono startup measurements, which is not related to the changes in the PR.

ivanpovazan
ivanpovazan approved these changes Jun 5, 2023
View reviewed changes
Copy link
Member

@ivanpovazan ivanpovazan left a comment

Choose a reason for hiding this comment

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

LGTM!
However, I think someone from the infra team should approve it as well, before merging.

@kotlarmilos
Copy link
Member Author

/cc: @LoopedBard3 @radical @akoeplinger

@kotlarmilos kotlarmilos requested a review from akoeplinger June 5, 2023 11:54
LoopedBard3
LoopedBard3 approved these changes Jun 5, 2023
View reviewed changes
Copy link
Member

@LoopedBard3 LoopedBard3 left a comment

Choose a reason for hiding this comment

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

This looks great! Looks good for merge presuming we have a test run of the pipeline with these changes 👍. I will likely also have to update the power-bi so will make a task to follow up on that.

@kotlarmilos kotlarmilos merged commit 578f8ac into dotnet:main Jun 5, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jul 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@LoopedBard3 LoopedBard3 LoopedBard3 approved these changes

@ivanpovazan ivanpovazan ivanpovazan approved these changes

@radical radical Awaiting requested review from radical

@akoeplinger akoeplinger Awaiting requested review from akoeplinger akoeplinger is a code owner

Assignees

@kotlarmilos kotlarmilos

Labels
area-Infrastructure-mono
Projects
None yet
Milestone
8.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@kotlarmilos @LoopedBard3 @ivanpovazan