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

[VMR] Use CopyFiles@2 task instead of manual copying #45479

Merged
merged 4 commits into from
Dec 16, 2024
Merged

[VMR] Use CopyFiles@2 task instead of manual copying #45479

merged 4 commits into from
Dec 16, 2024

Conversation

akoeplinger
Copy link
Member

Discussed in #44969

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Infrastructure untriaged Request triage from a team member labels Dec 16, 2024
@akoeplinger akoeplinger mentioned this pull request Dec 16, 2024
Merged
ViktorHofer
ViktorHofer reviewed Dec 16, 2024
View reviewed changes
Copy link
Member

@ViktorHofer ViktorHofer left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks great. The hasScenarioTestResults variable is now unset but read later. I'm unsure if we really need / want the variable. The task doesn't throw an error if it can't find any scenario test result files but I think it emits a warning.

@akoeplinger
Copy link
Member Author

akoeplinger commented Dec 16, 2024 •
edited
Loading

Yes good point, the PublishTestResult task will emit a warning which is noisy. I'll add back setting the hasScenarioTestResults.

@akoeplinger akoeplinger marked this pull request as ready for review December 16, 2024 16:41
@akoeplinger akoeplinger requested review from a team as code owners December 16, 2024 16:41
@akoeplinger akoeplinger mentioned this pull request Dec 16, 2024
Closed
@build-analysis build-analysis bot mentioned this pull request Dec 16, 2024
Open
@akoeplinger akoeplinger enabled auto-merge (squash) December 16, 2024 17:31
@ViktorHofer ViktorHofer disabled auto-merge December 16, 2024 18:34
@ViktorHofer
Copy link
Member

@akoeplinger the prebuilt-report folder shouldn't appear for non-source-only builds:

image

Also, I'm not sure if scenario-tests was at this location:

image

I don't really care about the scenario-tests location though.

@akoeplinger
Copy link
Member Author

Yeah I saw the prebuilt-report, will fix it.

The scenario-tests artifacts were there before.

@ViktorHofer ViktorHofer merged commit a98621f into main Dec 16, 2024
35 of 38 checks passed
@ViktorHofer ViktorHofer deleted the akoeplinger-patch-2 branch December 16, 2024 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ellahathaway ellahathaway ellahathaway approved these changes

@ViktorHofer ViktorHofer ViktorHofer approved these changes

Assignees
No one assigned
Labels
Area-Infrastructure untriaged Request triage from a team member
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@akoeplinger @ViktorHofer @ellahathaway