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.

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

[Helix] Move helix arm run to ci #13939

Merged
merged 2 commits into from
Sep 13, 2019
Merged

[Helix] Move helix arm run to ci #13939

merged 2 commits into from
Sep 13, 2019

Conversation

HaoK
Copy link
Member

@HaoK HaoK commented Sep 13, 2019

This should make helix passing on arm queues required for PR checks

@HaoK HaoK requested a review from a team as a code owner September 13, 2019 01:15
@@ -487,6 +487,26 @@ jobs:
path: artifacts/TestResults/
publishOnError: true

# Helix ARM64
Copy link
Member

Choose a reason for hiding this comment

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

@Pilchie @HaoK I thought the idea was to pare down the Helix pipeline to just the ARM builds and watch that for at least a few days before making it mandatory? Fine if this was the decision.

In any case, I'd prefer we weren't running the x64 Helix job until it's shaded less red.

Copy link
Member Author

@HaoK HaoK Sep 13, 2019

Choose a reason for hiding this comment

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

In the past week or so I haven't actually seen the arm tests fail while tweaking the helix stuff. I just took a brief manual look at the arm runs for the past few days and I haven't seen any real failures so I'm optimistic. Maybe we can try turning it on over the weekend and I can monitor things while its lower traffic, its easy to back out anyways if ends up blocking PRs.

Now that the mac queue is gone, and the log typo has been fixed I've actually seen consistent green helix checks since yesterday on my PRs!

Copy link
Member

Choose a reason for hiding this comment

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

Let's try this and see how it goes. We should also send a mail to the team describing the change, and who to talk to if there are issues.

@Pilchie Pilchie added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Sep 13, 2019
@@ -487,6 +487,26 @@ jobs:
path: artifacts/TestResults/
publishOnError: true

# Helix ARM64
Copy link
Member

Choose a reason for hiding this comment

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

Let's try this and see how it goes. We should also send a mail to the team describing the change, and who to talk to if there are issues.

@HaoK
Copy link
Member Author

HaoK commented Sep 13, 2019

Wish me luck, I told people to contact me directly if things blow up

@HaoK HaoK merged commit 7b0b298 into master Sep 13, 2019
@ghost ghost deleted the haok-require-arm branch September 13, 2019 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants