Skip to content

Add ProcessExitStatus class to System.Diagnostics.Process#124264

Merged
adamsitnik merged 10 commits intomainfrom
copilot/add-processexitstatus-class
Feb 13, 2026
Merged

Add ProcessExitStatus class to System.Diagnostics.Process#124264
adamsitnik merged 10 commits intomainfrom
copilot/add-processexitstatus-class

Conversation

Copy link
Contributor

Copilot AI commented Feb 11, 2026

Implementation Plan for ProcessExitStatus

  • Add ProcessExitStatus to reference file (ref/System.Diagnostics.Process.cs)
  • Add ProcessExitStatus implementation in src/System/Diagnostics/ProcessExitStatus.cs
  • Add XML documentation based on the approved API shape
  • Add basic unit tests for ProcessExitStatus
  • Build and test the changes
  • Address API review feedback - remove IEquatable and equality members
  • Change from struct to sealed class per approved API shape
  • Rename constructor parameter cancelled to canceled to match Canceled property
  • Clarify Signal property remarks for publicly constructible type
  • Improve Signal property docs to clarify it is a Unix-only concept

The API got approved in #123380 (comment)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

@copilot address my feedback

@dotnet-policy-service
Copy link
Contributor

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

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Copilot AI changed the title [WIP] Add ProcessExitStatus class to System.Diagnostics namespace Add ProcessExitStatus struct to System.Diagnostics.Process Feb 11, 2026
Copilot AI requested a review from adamsitnik February 11, 2026 12:40
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 11, 2026

@copilot address my feedback

Addressed in commit b291ae1. Removed IEquatable interface and all equality-related members to match the approved API shape.

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Copilot AI changed the title Add ProcessExitStatus struct to System.Diagnostics.Process Add ProcessExitStatus class to System.Diagnostics.Process Feb 11, 2026
Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

LGTM

@adamsitnik adamsitnik marked this pull request as ready for review February 11, 2026 13:01
Copilot AI review requested due to automatic review settings February 11, 2026 13:01
@adamsitnik adamsitnik enabled auto-merge (squash) February 11, 2026 13:10
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new System.Diagnostics.ProcessExitStatus public type to represent process termination state (exit code, optional POSIX signal, and cancellation/timeout termination), along with unit tests and project file wiring to compile/ship the new API.

Changes:

  • Added new public API ProcessExitStatus (ref + implementation) in System.Diagnostics.Process.
  • Wired the new source file into the product csproj and added a new unit test file.
  • Added unit tests validating constructor/property behavior for multiple parameter combinations.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj Includes the new ProcessExitStatusTests.cs test file in compilation.
src/libraries/System.Diagnostics.Process/tests/ProcessExitStatusTests.cs Adds unit tests for ProcessExitStatus constructor/property combinations.
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessExitStatus.cs Implements the new public ProcessExitStatus class with XML docs.
src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj Adds ProcessExitStatus.cs to the product compilation list.
src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs Adds the ProcessExitStatus public ref-surface declaration.

@stephentoub
Copy link
Member

@copilot please invoke the code-review skill and post your analysis/results as a comment on this PR

…rty name

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
auto-merge was automatically disabled February 11, 2026 13:46

Head branch was pushed to by a user without write access

Copilot AI requested a review from adamsitnik February 11, 2026 13:47
@stephentoub
Copy link
Member

@copilot please address the feedback

Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 11, 2026

@copilot please address the feedback

Addressed the remaining reviewer feedback — clarified the Signal property remarks from "This property is always null on Windows" to "When produced by Process APIs on Windows, this property is always null" since the type is publicly constructible (commit 9932cba). The cancelledcanceled rename was already done in 2f9f9f6.

Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

LGTM

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
adamsitnik and others added 2 commits February 13, 2026 09:13
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
Removed platform-specific remarks about exit codes.
@adamsitnik adamsitnik requested a review from jkotas February 13, 2026 08:18
Copy link
Member

@rzikm rzikm left a comment

Choose a reason for hiding this comment

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

LGTM

@adamsitnik adamsitnik merged commit e771b59 into main Feb 13, 2026
91 checks passed
@adamsitnik adamsitnik deleted the copilot/add-processexitstatus-class branch February 13, 2026 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants