Skip to content

RemoveOSGroup from the arcade - #4911

Merged
Anipik merged 1 commit into
dotnet:masterfrom
Anipik:OSgroup
Feb 25, 2020
Merged

RemoveOSGroup from the arcade#4911
Anipik merged 1 commit into
dotnet:masterfrom
Anipik:OSgroup

Conversation

@Anipik

@Anipik Anipik commented Feb 24, 2020

Copy link
Copy Markdown
Contributor

OSGroup is dotnet runtime specific property and should not be used in arcade.
The SharedFramework.sdk And Microsoft.Dotnet.CodeAnalysis are 2 projects that use this property.

SharedFramework.Sdk is used in -> runtime and dotnet\windowsDesktop
Microsoft.dotnet.codeanalysis is used in -> runtime, wpf, buildtools

CodeAnalysis change is restricted to pinvoke checking. I tested the change by building the runtime repo with these new packages.

Fixes #4907

@Anipik
Anipik requested review from ViktorHofer, dagood, ericstj and safern and removed request for dagood February 24, 2020 22:32
@ViktorHofer

Copy link
Copy Markdown
Member

As discussed offline, please prefer using $(OS) for Windows_NT and Unix: dotnet/msbuild#2468 (comment).


<!-- PInvokeChecker data files-->
<PropertyGroup Condition="'$(OSGroup)'=='Windows_NT'">
<PropertyGroup Condition="'$(OS)'=='Windows_NT'">

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: whitespace around the == operator :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I can fix this in another arcade PR :P I need this change to be in for the BuildOS change

@safern safern left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

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.

Replace OSGroup in Arcade with MsBuild::IsOSPlatform

4 participants