Skip to content

Make helix-inner-step send job warnaserror the same between windows and non-windows platforms#86729

Merged
LoopedBard3 merged 4 commits into
dotnet:mainfrom
LoopedBard3:MatchSendToHelixNoneWindowsToWindows
May 25, 2023
Merged

Make helix-inner-step send job warnaserror the same between windows and non-windows platforms#86729
LoopedBard3 merged 4 commits into
dotnet:mainfrom
LoopedBard3:MatchSendToHelixNoneWindowsToWindows

Conversation

@LoopedBard3
Copy link
Copy Markdown
Member

@LoopedBard3 LoopedBard3 commented May 24, 2023

This PR adds warnaserror false when using the send-to-helix-inner-step.yml on non-windows systems. warnaserror 0 is already being used for the same step for windows systems so this brings these two steps into parity.

@trylek Are you aware of any reason's to not use the same warnaserror values on non-windows systems as the windows system? Any thoughts on this change are appreciated!

Background: Our performance pipelines are getting warnings about some queues being removed soon which we are already dealing with. However, jobs targetting MacOS queues are having the removal warning populate as an error hiding the actual results from the jobs.

Test run here: https://dev.azure.com/dnceng/internal/_build/results?buildId=2187577&view=logs&j=8acf73da-ca1d-50e8-cd01-44779728784d&t=3c7febe5-578c-5a7c-1582-f61ec279ce08

@LoopedBard3 LoopedBard3 self-assigned this May 24, 2023
@ghost ghost added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label May 24, 2023
@LoopedBard3 LoopedBard3 added area-Infrastructure and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels May 24, 2023
@LoopedBard3 LoopedBard3 marked this pull request as ready for review May 24, 2023 21:50
@ghost
Copy link
Copy Markdown

ghost commented May 24, 2023

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Issue Details

This PR adds warnaserror false when using the send-to-helix-inner-step.yml on non-windows systems. warnaserror 0 is already being used for the same step for windows systems so this brings these two steps into parity.

@trylek Are you aware of any reason's to not use the same warnaserror values on non-windows systems as the windows system? Any thoughts on this change are appreciated!

Background: Our performance pipelines are getting warnings about some queues being removed soon which we are already dealing with. However, jobs targetting MacOS queues are having the removal warning populate as an error hiding the actual results from the jobs.

Author: LoopedBard3
Assignees: LoopedBard3
Labels:

area-Infrastructure

Milestone: -

Copy link
Copy Markdown
Member

@trylek trylek left a comment

Choose a reason for hiding this comment

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

I don't recall anything fundamental off the top of my head, your change looks good to me, perhaps it might be useful to double-check with someone on Mono like @naricc if there's not a subtle catch that might need this behavioral difference.

Copy link
Copy Markdown
Contributor

@caaavik-msft caaavik-msft left a comment

Choose a reason for hiding this comment

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

It seems before this, warnaserror was set to false for the Windows build step and the non-Windows restore step. This PR only adds it to the non-Windows build step, should we also set it for the Windows restore step while here to ensure both are in sync?

@LoopedBard3
Copy link
Copy Markdown
Member Author

It seems before this, warnaserror was set to false for the Windows build step and the non-Windows restore step. This PR only adds it to the non-Windows build step, should we also set it for the Windows restore step while here to ensure both are in sync?

That makes sense. Added warnaserror 0 to the windows restore. Perf pipeline test run: https://dev.azure.com/dnceng/internal/_build/results?buildId=2188271&view=results

@LoopedBard3
Copy link
Copy Markdown
Member Author

Test run ran as expected, merging.

@LoopedBard3 LoopedBard3 merged commit 66644e3 into dotnet:main May 25, 2023
@LoopedBard3 LoopedBard3 deleted the MatchSendToHelixNoneWindowsToWindows branch May 25, 2023 19:46
@ghost ghost locked as resolved and limited conversation to collaborators Jun 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants