Skip to content

Conversation

@sfoslund
Copy link
Member

@sfoslund sfoslund commented Jul 17, 2020

Fixes #12456

@sfoslund sfoslund requested review from dsplaisted and wli3 July 20, 2020 15:24

<PropertyGroup>
<TargetPlatformVersionSupported Condition="'$(TargetPlatformVersionSupported)' == '' and '@(ValidTargetPlatformVersion)' != ''" >true</TargetPlatformVersionSupported>
<ValidTargetPlatformVersions Condition="'@(SupportedTargetPlatform)' != ''" >@(SupportedTargetPlatform, ', ')</ValidTargetPlatformVersions>
Copy link
Member

Choose a reason for hiding this comment

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

I think these should be joined by newlines, because comma isn't the right separator character for all locales. Confirm with @wli3, if he thinks this is OK then go ahead with it.

Copy link

Choose a reason for hiding this comment

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

Comma is ok. But whitespace is better

FormatArguments="$(MinimumOSPlatform);$(TargetPlatformVersion)"/>
</Target>

<Target Name="_CheckForInvalidTargetPlatformVersion"
Copy link
Member

Choose a reason for hiding this comment

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

How does this interact with the invalid platform error? If you have an invalid platform and specify a TargetPlatformVersion, then we should only give you the invalid platform error, and not the invalid platform version error. Can you add a test case covering this?

@sfoslund
Copy link
Member Author

Closing in favor of #12613

@sfoslund sfoslund closed this Jul 23, 2020
@sfoslund sfoslund deleted the SupportedPlatformVersions branch July 24, 2020 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Generate error if TargetPlatformVersion is not a valid version when targeting Windows

3 participants