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

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

Comments

@dmitshur
Copy link
Member

@dmitshur dmitshur commented May 31, 2020

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

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Jun 24, 2020

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

@stamblerre stamblerre closed this Jun 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.