Adds FreeBSD support to S.RT.I.RI #2068
Conversation
| @@ -7,19 +7,21 @@ public struct OSPlatform : IEquatable<OSPlatform> | |||
| { | |||
| private readonly string _osPlatform; | |||
|
|
|||
| private const string WindowsName = "WINDOWS"; | |||
| private const string FreeBSDName = "FreeBSD"; | |||
There was a problem hiding this comment.
I thought the decision was for the strings to be all upper case, no? @Priya91?
|
One additional comment on the tests, but otherwise LGTM. cc: @ellismg |
|
LGTM |
|
Did the buildtools commits already get merged? I can do the corefx update soon if so. |
|
@mmitche, no, they haven't been written yet. |
|
@mmitche, working on it. :) |
ghost
mentioned this pull request
|
|
||
| public static OSPlatform Windows | ||
| public static OSPlatform FreeBSD |
There was a problem hiding this comment.
@ellismg just pointed out to me that this is public. It shouldn't be (in fact we should just remove it for now). As agreed upon at #1999 (comment), we're not going to add public surface area like this until more progress is made.
There was a problem hiding this comment.
Should we also remove RuntimeInformation.FreeBSD.cs (public class)?
There was a problem hiding this comment.
No, that's not new surface area... it's an implementation detail and is the entire point of your change 😉
There was a problem hiding this comment.
Aha, yes 😄
I have removed and amended the commit.
|
Would you remind removing the "xplat: " token in the commit title when you rebase? |
System.Runtime.InteropServices.RuntimInformation. Issue-URL: #1626 PR-URL: #2068
ghost
changed the title
|
LGTM. Matt? |
|
LGTM |
Adds FreeBSD support to S.RT.I.RI
| Assert.False(RuntimeInformation.IsOSPlatform(OSPlatform.Linux)); | ||
| Assert.False(RuntimeInformation.IsOSPlatform(OSPlatform.OSX)); | ||
| } | ||
|
|
||
| [Fact, PlatformSpecific(PlatformID.Linux)] |
There was a problem hiding this comment.
@jasonwilliams200OK Where are the CheckFreeBSD() tests?
[Fact, PlatformSpecific(PlatformID.FreeBSD)]
public void CheckFreeBSD()
{
// yada
}
Refs: dotnet/corefx#2068 PR-URL: dotnet#180
Refs: dotnet/corefx#2068 PR-URL: #180
System.Runtime.InteropServices.RuntimInformation. Issue-URL: dotnet/corefx#1626 PR-URL: dotnet/corefx#2068 Commit migrated from dotnet/corefx@64b7f87
Adds FreeBSD support to S.RT.I.RI Commit migrated from dotnet/corefx@f9b54a8
System.Runtime.InteropServices.RuntimInformation.
Issue-URL: #1626
R=@stephentoub,@josteink,@janhenke,@ghuntley