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

all: add godoc link everywhere possible. #1112

Closed
wants to merge 1 commit into from

Conversation

ccoVeille
Copy link
Contributor

zero code changes, only godoc update

zero code changes, only godoc update
@ccoVeille ccoVeille changed the title chore: add godoc link everywhere possible. all: add godoc link everywhere possible. Dec 2, 2024
Copy link
Owner

@mvdan mvdan left a comment

Choose a reason for hiding this comment

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

Thanks - merging most of these changes to master.

@@ -457,7 +457,7 @@ func stdinFile(r io.Reader) (*os.File, error) {
// Note that providing a non-nil standard input other than [os.File] will require
// an [os.Pipe] and spawning a goroutine to copy into it,
// as an [os.File] is the only way to share a reader with subprocesses.
// See [os/exec.Cmd.Stdin].
// See os/[exec.Cmd.Stdin].
Copy link
Owner

Choose a reason for hiding this comment

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

I'm undoing this change as it seems unnecessary and makes the linking less consistent.

@@ -947,7 +947,7 @@ func (r *Runner) readLine(ctx context.Context, raw bool) ([]byte, error) {
// [cancelreader.NewReader] may fail under some circumstances, such as r.stdin being
// a regular file on Linux, in which case epoll returns an "operation not permitted" error
// given that regular files can always be read immediately. Polling them makes no sense.
// As such, if cancelreader fails, fall back to no cancellation, meaning this is best-effort.
// As such, if [cancelreader] fails, fall back to no cancellation, meaning this is best-effort.
Copy link
Owner

Choose a reason for hiding this comment

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

this code is gone from master now; removing this change.

@@ -209,7 +210,7 @@ func LookPath(env expand.Environ, file string) (string, error) {
return LookPathDir(env.Get("PWD").String(), env, file)
}

// LookPathDir is similar to [os/exec.LookPath], with the difference that it uses the
// LookPathDir is similar to os/[exec.LookPath], with the difference that it uses the
Copy link
Owner

Choose a reason for hiding this comment

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

also reverting this one.

@@ -54,7 +54,7 @@ func TestRunnerTerminalStdIO(t *testing.T) {

r, _ := interp.New(interp.StdIO(secondaryReader, secondary, secondary))
go func() {
// To mimic os/exec.Cmd.Start, use a goroutine.
// To mimic os/[exec.Cmd.Start], use a goroutine.
Copy link
Owner

Choose a reason for hiding this comment

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

tweaking to quote the entire qualified name, like the others.

@@ -279,7 +279,7 @@ func decodeValue(val reflect.Value, enc any) error {
fval := val.FieldByName(name)
switch name {
case "Type", "Pos", "End":
// Type is already used above. Pos and End came from method calls.
// Type is already used above. [syntax.Node.Pos] and [syntax.Node.End] came from method calls.
Copy link
Owner

Choose a reason for hiding this comment

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

these refer to the JSON key names, not the methods. undoing these changes.

@mvdan mvdan closed this in a56e337 Jan 2, 2025
@ccoVeille ccoVeille deleted the add-godoc-links branch January 2, 2025 16:40
@ccoVeille
Copy link
Contributor Author

Thanks - merging most of these changes to master.

Thanks for the review,and telling why you excluded some of them

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants