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

Annotate unsupported APIs in System.Diagnostics.Process #57120

Merged
merged 7 commits into from
Aug 11, 2021

Conversation

adamsitnik
Copy link
Member

fixes #57090

@dotnet-issue-labeler
Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost
Copy link

ghost commented Aug 10, 2021

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

Issue Details

fixes #57090

Author: adamsitnik
Assignees: -
Labels:

area-System.Diagnostics.Process

Milestone: 6.0.0

@@ -72,9 +72,5 @@ internal static void ThrowIfRemoteMachine(string machineName)
throw new PlatformNotSupportedException(SR.RemoteMachinesNotSupported);
}
}
public static IntPtr GetMainWindowHandle(int processId)
Copy link
Member Author

Choose a reason for hiding this comment

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

as pointed by @buyaa-n in #57090 (comment) it turned out to be dead code

@@ -528,6 +528,8 @@ public string ProcessName
/// </devdoc>
public IntPtr ProcessorAffinity
{
[SupportedOSPlatform("windows")]
[SupportedOSPlatform("linux")]
Copy link
Member Author

Choose a reason for hiding this comment

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

Windows and Linux support it, everything else does not.

@@ -88,6 +88,9 @@ public bool PriorityBoostEnabled
/// </devdoc>
public ThreadPriorityLevel PriorityLevel
{
[SupportedOSPlatform("windows")]
[SupportedOSPlatform("linux")]
[SupportedOSPlatform("freebsd")]
Copy link
Member Author

Choose a reason for hiding this comment

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

Windows, Linux and FreeBSD support it, everything else does not.

Copy link
Member

@jeffhandley jeffhandley left a comment

Choose a reason for hiding this comment

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

I concur with @stephentoub that property annotations should be on the property instead of the accessors, but otherwise approved.

I'm glad to see this get done.

@ghost
Copy link

ghost commented Aug 10, 2021

Hello @adamsitnik!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@jeffhandley
Copy link
Member

Ah, I overlooked that the getter had different annotations than the setter. Looks good.

Copy link
Member

@buyaa-n buyaa-n left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

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.

Annotate unsupported APIs in System.Diagnostics.Process
4 participants