Skip to content

Conversation

@radekdoulik
Copy link
Member

@radekdoulik radekdoulik commented Jan 13, 2021

Context: Fixes #777

Looks like this define will not be added, so check for NET instead.

Context: dotnet#777

Looks like this define will not be added, so check for `NET` instead.
@radekdoulik radekdoulik requested a review from jpobst January 13, 2021 13:56
@jpobst
Copy link
Contributor

jpobst commented Jan 13, 2021

Due to a lot of confusion and concern we ended up not shipping this design.

I think this sentence refers to the alternative proposed in the QA question.

I think the actual issue is that NET5_0_OR_GREATER hasn't been added to .NET 5 yet given that this design was accepted 2 months ago. I think we will need to do #if !NET5_0 AND !NET5_0_OR_GREATER (pseudo-code).

@radekdoulik
Copy link
Member Author

I think we will need to do #if !NET5_0 AND !NET5_0_OR_GREATER (pseudo-code).

That would not work, as on net6 we have these defines now (plus non-related other defines): NET;NET6_0;NETCOREAPP.

So I think checking for the NET should be fine for now.

@jpobst
Copy link
Contributor

jpobst commented Jan 13, 2021

Yeah, looks like that should work. The way I originally read the document made me think NET would be defined for net472 TFM.

@jonpryor jonpryor merged commit a33084b into dotnet:master Jan 14, 2021
@jpobst jpobst added this to the 11.2 (16.10 / 8.10) milestone Feb 2, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Do not generate SupportedOSPlatformAttribute type on net6?

3 participants