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.

Sign up for GitHub

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

Jump to bottom

x/tools/internal/gocommand: documentation doesn't state what Run{,Piped,Raw} do and what they returns #39333

Closed
dmitshur opened this issue May 31, 2020 · 1 comment
Closed

x/tools/internal/gocommand: documentation doesn't state what Run{,Piped,Raw} do and what they returns #39333

dmitshur opened this issue May 31, 2020 · 1 comment
Labels
Documentation FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Backlog

Comments

@dmitshur
Copy link
Contributor

dmitshur commented May 31, 2020 •
edited

There are 3 exported identifiers with the word "Run" in package golang.org/x/tools/internal/gocommand. None of them say what the identifiers do and what they return. Each one says "it's like this other Run variant". This makes it hard to know how to use the API of gocommand package.

// RunPiped is like Run, but relies on the given stdout/stderr
func (i *Invocation) RunPiped(ctx context.Context, stdout, stderr io.Writer) error

// Run calls Runner.RunRaw, serializing requests if they fight over go.mod changes.
func (runner *Runner) Run(ctx context.Context, inv Invocation) (*bytes.Buffer, error)

// RunRaw calls Invocation.runRaw, serializing requests if they fight over go.mod changes.
func (runner *Runner) RunRaw(ctx context.Context, inv Invocation) (*bytes.Buffer, *bytes.Buffer, error, error)

Documentation for Invocation.runRaw isn't readily visible because it's an unexported identifier:

// RunRaw is like RunPiped, but also returns the raw stderr and error for callers
// that want to do low-level error handling/recovery.
func (i *Invocation) runRaw(ctx context.Context) (stdout *bytes.Buffer, stderr *bytes.Buffer, friendlyError error, rawError error)

It has named return values that help.

The Runner type is documented as follows:

// An Runner will run go command invocations and serialize them if it sees a concurrency error.

Perhaps that should be made more visible on the methods, and their *bytes.Buffer and error results can be given names like stdout, stderr to help clarify what's what.

/cc @heschik
@dmitshur dmitshur added Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels May 31, 2020
@dmitshur dmitshur added this to the Backlog milestone May 31, 2020
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label May 31, 2020
@stamblerre
Copy link
Contributor

There have been a number of changes to this package recently, and the documentation has been improved as a result. Closing.

@golang golang locked and limited conversation to collaborators Jun 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Documentation FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
3 participants
@dmitshur @stamblerre @gopherbot