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

Remove the logic duplication for iOS case #46189

Merged
merged 4 commits into from
Dec 18, 2020

Conversation

MaximLipnin
Copy link
Contributor

Addresses #46156 (comment)

@ghost
Copy link

ghost commented Dec 17, 2020

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

Issue Details

Addresses #46156 (comment)

Author: MaximLipnin
Assignees: -
Labels:

area-Infrastructure-libraries

Milestone: -

Copy link
Member

@safern safern left a comment

Choose a reason for hiding this comment

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

LGTM. If you wanted to leave the runonly path outside of TestArchiveTestsRoot you could use an ItemDefinitionGroup to define the common properties for the item group. i.e:

<ItemDefinitionGroup Condition="'$(TargetOS)' == 'iOS' or '$(TargetOS)' == 'tvOS'">
  <MyItem>
    <SharedMetadata>This is shared</SharedMetadata>
  </MyItem>
</ItemDefinitionGroup>

<ItemGroup Condition="'$(TargetOS)' == 'iOS' or '$(TargetOS)' == 'tvOS'">
  <MyItem Include="Item1" />
  <MyItem Include="Item2">
    <SpecificMetadata>MetadataForItem2</SpecificMetadata>
  </MyItem>
</ItemGroup>

@MaximLipnin
Copy link
Contributor Author

Perhaps, it makes sense to try this variant because the checking for substring looks like a less obvious option here :)

@akoeplinger
Copy link
Member

The dotnet-linker-tests and runtime-dev-innerloop are green on AzDO but the status didn't make it to GitHub. Merging.

@akoeplinger akoeplinger merged commit ac50589 into dotnet:master Dec 18, 2020
@MaximLipnin MaximLipnin deleted the ios_helix_cleanup branch December 18, 2020 13:07
@ghost ghost locked as resolved and limited conversation to collaborators Jan 17, 2021
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.

5 participants