Skip to content

Single source of truth for System.Diagnostics.Processes annotations and docs - #68852

Closed
adamsitnik wants to merge 2 commits into
dotnet:mainfrom
adamsitnik:processFacade
Closed

adamsitnik wants to merge 2 commits into
dotnet:mainfrom
adamsitnik:processFacade

Conversation

@adamsitnik

Copy link
Copy Markdown
Member

In order to avoid API docs and compat inconsistencies (#68834 (review)), we could use the facade pattern. The only disadvantage I can see right now is having an additional method invocation. But since these methods usually perform a very expensive sys-call, I don't believe that it matters.

I took ProcessThread and experimented with it and got 316 deletions and 114 additions.

@ericstj @stephentoub @jozkee please let me know if you believe that this is the right direction. If it is, I am going to refactor the rest of the System.Diagnostics.Processes APIs.

@ghost

ghost commented May 4, 2022

Copy link
Copy Markdown

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

Issue Details

In order to avoid API docs and compat inconsistencies (#68834 (review)), we could use the facade pattern. The only disadvantage I can see right now is having an additional method invocation. But since these methods usually perform a very expensive sys-call, I don't believe that it matters.

I took ProcessThread and experimented with it and got 316 deletions and 114 additions.

@ericstj @stephentoub @jozkee please let me know if you believe that this is the right direction. If it is, I am going to refactor the rest of the System.Diagnostics.Processes APIs.

Author: adamsitnik
Assignees: adamsitnik
Labels:

area-System.ServiceProcess

Milestone: -

@ghost

ghost commented May 4, 2022

Copy link
Copy Markdown

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

Issue Details

In order to avoid API docs and compat inconsistencies (#68834 (review)), we could use the facade pattern. The only disadvantage I can see right now is having an additional method invocation. But since these methods usually perform a very expensive sys-call, I don't believe that it matters.

I took ProcessThread and experimented with it and got 316 deletions and 114 additions.

@ericstj @stephentoub @jozkee please let me know if you believe that this is the right direction. If it is, I am going to refactor the rest of the System.Diagnostics.Processes APIs.

Author: adamsitnik
Assignees: adamsitnik
Labels:

area-System.Diagnostics.Process

Milestone: -

@ericstj

ericstj commented May 4, 2022

Copy link
Copy Markdown
Member

It'd be handy if there was such a thing as "partial properties" similar to methods, but it seems to be explicitly not supported: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/partial-method :(

This makes sense to me since all of these are likely to be method calls anyway since they are not auto-properties. I bet the JIT will just inline these anyway.

@adamsitnik

Copy link
Copy Markdown
Member Author

I am going to finish it once I am back from honeymoon

@adamsitnik adamsitnik closed this May 18, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jun 17, 2022
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.

3 participants