Fix documentation throughout the codebase and enforce that all public symbols have documentation. #163
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
iCharlesHu
requested review from
jakepetroules,
parkera,
itingliu and
cthielen
… symbols have documentation.
iCharlesHu
force-pushed
the
charles/documentation-update
branch
from
eb8b169 to
69920c1
Compare
heckj
reviewed
Aug 29, 2025
Sources/Subprocess/IO/Input.swift
Outdated
Comment on lines
36
to
37
| /// InputProtocol defines the `write(with:)` method that a type must | ||
| /// implement to serve as the input source for a subprocess. |
There was a problem hiding this comment.
Suggested change
| /// InputProtocol defines the `write(with:)` method that a type must | |
| /// implement to serve as the input source for a subprocess. | |
| /// InputProtocol defines a type that serves as the input source for a subprocess. | |
| /// | |
| /// The protocol defines the `write(with:)` method that a type must | |
| /// implement to serve as the input source. |
In general, abstracts for symbols should be a single sentence that doesn't use code voice (symbol references or monospaced fonts). Breaking it up with a line break in between drops the detail about the method that must be provided (which the protocol conformance enforces) into the discussion section for the type. The abstract (that first line) is propagated and shown alongside the content anywhere the symbol is referenced.
Comment on lines
44
to
47
| /// A concrete input type for subprocesses that indicates the absence | ||
| /// of input to the subprocess. On Unix-like systems, `NoInput` | ||
| /// redirects the standard input of the subprocess to /dev/null, | ||
| /// while on Windows, it redirects to `NUL`. |
There was a problem hiding this comment.
Suggested change
| /// A concrete input type for subprocesses that indicates the absence | |
| /// of input to the subprocess. On Unix-like systems, `NoInput` | |
| /// redirects the standard input of the subprocess to /dev/null, | |
| /// while on Windows, it redirects to `NUL`. | |
| /// A concrete input type for subprocesses that indicates the absence | |
| /// of input to the subprocess. | |
| /// | |
| /// On Unix-like systems, `NoInput` redirects the standard input of the subprocess to /dev/null, | |
| /// while on Windows, it redirects to `NUL`. |
Sources/Subprocess/IO/Input.swift
Outdated
Comment on lines
62
to
63
| /// Asynchronously write the input to the subprocess using the | ||
| /// write file descriptor |
There was a problem hiding this comment.
Suggested change
| /// Asynchronously write the input to the subprocess using the | |
| /// write file descriptor | |
| /// Asynchronously write the input to the subprocess that uses the | |
| /// write file descriptor. |
Sources/Subprocess/IO/Input.swift
Outdated
Comment on lines
71
to
74
| /// A concrete input type for subprocesses that reads input from a | ||
| /// specified FileDescriptor. Developers have the option to | ||
| /// instruct the Subprocess to automatically close the provided | ||
| /// FileDescriptor after the subprocess is spawned. |
There was a problem hiding this comment.
Suggested change
| /// A concrete input type for subprocesses that reads input from a | |
| /// specified FileDescriptor. Developers have the option to | |
| /// instruct the Subprocess to automatically close the provided | |
| /// FileDescriptor after the subprocess is spawned. | |
| /// A concrete input type for subprocesses that reads input from a specified FileDescriptor. | |
| /// | |
| /// Developers have the option to instruct the Subprocess to automatically close the provided | |
| /// FileDescriptor after the subprocess is spawned. |
Sources/Subprocess/IO/Input.swift
Outdated
Comment on lines
94
to
95
| /// Asynchronously write the input to the subprocess using the | ||
| /// write file descriptor |
There was a problem hiding this comment.
Suggested change
| /// Asynchronously write the input to the subprocess using the | |
| /// write file descriptor | |
| /// Asynchronously write the input to the subprocess that use the | |
| /// write file descriptor. |
Sources/Subprocess/IO/Input.swift
Outdated
Comment on lines
119
to
120
| /// Asynchronously write the input to the subprocess using the | ||
| /// write file descriptor |
There was a problem hiding this comment.
Suggested change
| /// Asynchronously write the input to the subprocess using the | |
| /// write file descriptor | |
| /// Asynchronously write the input to the subprocess that use the | |
| /// write file descriptor. |
heckj
reviewed
Sep 2, 2025
Sources/Subprocess/IO/Input.swift
Outdated
| /// - Parameter span: The span to write | ||
| /// - Returns number of bytes written | ||
| #if SubprocessSpan | ||
| /// Write a `RawSpan` to the standard input of the subprocess. |
There was a problem hiding this comment.
Suggested change
| /// Write a `RawSpan` to the standard input of the subprocess. | |
| /// Write a raw span to the standard input of the subprocess. | |
| /// |
Sources/Subprocess/IO/Input.swift
Outdated
| #endif | ||
|
|
||
| /// Write a StringProtocol to the standard input of the subprocess. | ||
| /// Write a `StringProtocol` to the standard input of the subprocess. |
There was a problem hiding this comment.
Suggested change
| /// Write a `StringProtocol` to the standard input of the subprocess. | |
| /// Write a type that conforms to StringProtocol to the standard input of the subprocess. |
Sources/Subprocess/IO/Output.swift
Outdated
Comment on lines
26
to
30
| /// `OutputProtocol` specifies the set of methods that a type must implement to | ||
| /// serve as the output target for a subprocess. Instead of developing custom | ||
| /// implementations of `OutputProtocol`, it is recommended to utilize the | ||
| /// default implementations provided by the `Subprocess` library to specify the | ||
| /// output handling requirements. |
There was a problem hiding this comment.
Suggested change
| /// `OutputProtocol` specifies the set of methods that a type must implement to | |
| /// serve as the output target for a subprocess. Instead of developing custom | |
| /// implementations of `OutputProtocol`, it is recommended to utilize the | |
| /// default implementations provided by the `Subprocess` library to specify the | |
| /// output handling requirements. | |
| /// Output protocol specifies the set of methods that a type must implement to | |
| /// serve as the output target for a subprocess. | |
| /// Instead of developing custom implementations of `OutputProtocol`, use the | |
| /// default implementations provided by the `Subprocess` library to specify the | |
| /// output handling requirements. |
Sources/Subprocess/IO/Output.swift
Outdated
Comment on lines
51
to
55
| /// A concrete `Output` type for subprocesses that indicates that the | ||
| /// `Subprocess` should not collect or redirect output from the child | ||
| /// process. On Unix-like systems, `DiscardedOutput` redirects the | ||
| /// standard output of the subprocess to `/dev/null`, while on Windows, | ||
| /// redirects the output to `NUL`. |
There was a problem hiding this comment.
Suggested change
| /// A concrete `Output` type for subprocesses that indicates that the | |
| /// `Subprocess` should not collect or redirect output from the child | |
| /// process. On Unix-like systems, `DiscardedOutput` redirects the | |
| /// standard output of the subprocess to `/dev/null`, while on Windows, | |
| /// redirects the output to `NUL`. | |
| /// A concrete output type for subprocesses that indicates that the | |
| /// subprocess should not collect or redirect output from the child | |
| /// process. | |
| /// On Unix-like systems, `DiscardedOutput` redirects the | |
| /// standard output of the subprocess to `/dev/null`, while on Windows, | |
| /// redirects the output to `NUL`. |
Sources/Subprocess/IO/Output.swift
Outdated
| /// standard output of the subprocess to `/dev/null`, while on Windows, | ||
| /// redirects the output to `NUL`. | ||
| public struct DiscardedOutput: OutputProtocol { | ||
| /// The output type for this output option |
There was a problem hiding this comment.
Suggested change
| /// The output type for this output option | |
| /// The type for the output. |
| return self.description(withIndent: 0) | ||
| } | ||
|
|
||
| /// A textual representation of this `PlatformOptions`. |
There was a problem hiding this comment.
Suggested change
| /// A textual representation of this `PlatformOptions`. | |
| /// A debug oriented textual representation of the platform options. |
| // MARK: - ProcessIdentifier | ||
|
|
||
| /// A platform independent identifier for a Subprocess. | ||
| /// A platform-independent identifier for a Subprocess |
There was a problem hiding this comment.
Suggested change
| /// A platform-independent identifier for a Subprocess | |
| /// A platform-independent identifier for a Subprocess. |
| /// The platform specific process identifier value | ||
| public let value: pid_t | ||
|
|
||
| /// Initialize a `ProcessIdentifier` with the given value |
There was a problem hiding this comment.
Suggested change
| /// Initialize a `ProcessIdentifier` with the given value | |
| /// Initialize a process identifier with the value you provide. |
| } | ||
|
|
||
| extension ProcessIdentifier: CustomStringConvertible, CustomDebugStringConvertible { | ||
| /// A textual representation of this `ProcessIdentifier`. |
There was a problem hiding this comment.
Suggested change
| /// A textual representation of this `ProcessIdentifier`. | |
| /// A textual representation of the process identifier. |
| /// A textual representation of this `ProcessIdentifier`. | ||
| public var description: String { "\(self.value)" } | ||
|
|
||
| /// A textual representation of this `ProcessIdentifier`. |
There was a problem hiding this comment.
Suggested change
| /// A textual representation of this `ProcessIdentifier`. | |
| /// A debug-oriented textual representation of the process identifier. |
heckj
reviewed
Sep 2, 2025
| // MARK: - ProcessIdentifier | ||
|
|
||
| /// A platform independent identifier for a Subprocess. | ||
| /// A platform-independent identifier for a Subprocess |
There was a problem hiding this comment.
Suggested change
| /// A platform-independent identifier for a Subprocess | |
| /// A platform-independent identifier for a subprocess. |
| public let value: pid_t | ||
|
|
||
| #if os(Linux) || os(Android) || os(FreeBSD) | ||
| /// Process file Descriptor (pidfd) for the running execution |
There was a problem hiding this comment.
Suggested change
| /// Process file Descriptor (pidfd) for the running execution | |
| /// The process file descriptor (pidfd) for the running execution. |
| } | ||
|
|
||
| extension ProcessIdentifier: CustomStringConvertible, CustomDebugStringConvertible { | ||
| /// A textual representation of this `ProcessIdentifier`. |
There was a problem hiding this comment.
Suggested change
| /// A textual representation of this `ProcessIdentifier`. | |
| /// A textual representation of the process identifier. |
| /// A textual representation of this `ProcessIdentifier`. | ||
| public var description: String { "\(self.value)" } | ||
|
|
||
| /// A textual representation of this `ProcessIdentifier`. |
There was a problem hiding this comment.
Suggested change
| /// A textual representation of this `ProcessIdentifier`. | |
| /// A debug-oriented textual representation of the process identifier. |
| /// Always ends in sending a `.kill` signal at the end. | ||
| public var teardownSequence: [TeardownStep] = [] | ||
|
|
||
| /// Initialize a `PlatformOptions` with default options. |
There was a problem hiding this comment.
Suggested change
| /// Initialize a `PlatformOptions` with default options. | |
| /// Create platform options with the default values. |
Sources/Subprocess/Result.swift
Outdated
| public let processIdentifier: ProcessIdentifier | ||
| /// The termination status of the executed subprocess | ||
| public let terminationStatus: TerminationStatus | ||
| /// The captured standard output of the executed subprocess |
There was a problem hiding this comment.
Suggested change
| /// The captured standard output of the executed subprocess | |
| /// The captured standard output of the executed subprocess. |
Sources/Subprocess/Result.swift
Outdated
|
|
||
| extension CollectedResult: CustomStringConvertible | ||
| where Output.OutputType: CustomStringConvertible, Error.OutputType: CustomStringConvertible { | ||
| /// A textual representation of this `CollectedResult`. |
There was a problem hiding this comment.
Suggested change
| /// A textual representation of this `CollectedResult`. | |
| /// A textual representation of the collected result. |
Sources/Subprocess/Result.swift
Outdated
|
|
||
| extension CollectedResult: CustomDebugStringConvertible | ||
| where Output.OutputType: CustomDebugStringConvertible, Error.OutputType: CustomDebugStringConvertible { | ||
| /// A textual representation of this `CollectedResult`. |
There was a problem hiding this comment.
Suggested change
| /// A textual representation of this `CollectedResult`. | |
| /// A debug-oriented textual representation of the collected result. |
Sources/Subprocess/Result.swift
Outdated
| extension ExecutionResult: Hashable where Result: Hashable {} | ||
|
|
||
| extension ExecutionResult: CustomStringConvertible where Result: CustomStringConvertible { | ||
| /// A textual representation of this `ExecutionResult`. |
There was a problem hiding this comment.
Suggested change
| /// A textual representation of this `ExecutionResult`. | |
| /// A textual representation of the execution result. |
Sources/Subprocess/Result.swift
Outdated
| } | ||
|
|
||
| extension ExecutionResult: CustomDebugStringConvertible where Result: CustomDebugStringConvertible { | ||
| /// A textual representation of this `ExecutionResult`. |
There was a problem hiding this comment.
Suggested change
| /// A textual representation of this `ExecutionResult`. | |
| /// A debug-oriented textual representation of this execution result. |
|
Thanks so much for looking into this @heckj !! |
Co-authored-by: Joseph Heck <j_heck@apple.com>
iCharlesHu
force-pushed
the
charles/documentation-update
branch
from
138fcdd to
7b577a7
Compare
jakepetroules
approved these changes
Sep 2, 2025
iCharlesHu
force-pushed
the
charles/documentation-update
branch
from
762dbc1 to
cdb9e21
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR improves documentation throughout the code base. It also turns on
AllPublicDeclarationsHaveDocumentationto enforce that all public symbols have proper documentation.While I was here
I noticed the
Documentationoption isn't showing up forswift-subprocesson Swift Package Index. Hopefully, the edit to.spy.ymlwill fix that.