Skip to content

Conversation

@sebastienros
Copy link
Member

No description provided.

@sebastienros sebastienros requested a review from HaoK May 6, 2020 22:03
@ghost ghost added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label May 6, 2020
<ItemGroup Condition="'$(IsWindowsOnlyTest)' != 'true' AND '$(TargetArchitecture)' == 'x64' AND '$(IsHelixDaily)' == 'true'">
<HelixAvailableTargetQueue Include="Windows.7.Amd64.Open" Platform="Windows" />
<!-- Temporarily ignoring win7 builds as they are failing on random tests -->
<!--HelixAvailableTargetQueue Include="Windows.7.Amd64.Open" Platform="Windows" /-->
Copy link
Member

Choose a reason for hiding this comment

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

Maybe just delete the line and file an issue?

Copy link
Member Author

Choose a reason for hiding this comment

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

I can also file an issue maybe, even better for visibility, unless you tell me that triage is super efficient and you won't miss it, then I'll trust you.

Copy link
Member

Choose a reason for hiding this comment

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

Well we didn't comment out win8 and there's no issue for that one, so maybe add back win8 then? just trying to be consistent :)

Copy link
Member

Choose a reason for hiding this comment

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

In a commented form I mean for win8

Copy link
Contributor

Choose a reason for hiding this comment

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

/fyi #21421 covers restoring the win8 queue. I plan to address this today or tomorrow

Copy link
Contributor

Choose a reason for hiding this comment

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

Because we haven't tested on win8 w/ the latest SDK, don't restore it unless it gets run in PR validation builds or you hack the PR to include it in your builds.

Copy link
Contributor

Choose a reason for hiding this comment

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

But, please, please, please avoid commented-out code. Just delete.

Copy link
Member Author

Choose a reason for hiding this comment

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

Created the issue. Might require some better labels and other metadata

@sebastienros sebastienros merged commit c836a3a into master May 7, 2020
@sebastienros sebastienros deleted the sebros/win7 branch May 7, 2020 01:28
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.

5 participants