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

doc: missing documentation of quoting the URL of url.Errors in go1.14 release notes #37614

Closed
fraenky8 opened this issue Mar 3, 2020 · 7 comments
Labels
Documentation FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@fraenky8
Copy link

fraenky8 commented Mar 3, 2020

What version of Go are you using (go version)?

$ go version
go version go1.14 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/fraenky8/Library/Caches/go-build"
GOENV="/Users/fraenky8/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOOS="darwin"
GOPATH="/Users/fraenky8/Coding/Go"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.14/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.14/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/8v/jgh2dzcn62n0lfyws13mc_rw0000gp/T/go-build103321297=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Print a url.Error:

err := &url.Error{
	Op:  "Get",
	URL: "http://localhost/test/path",
	Err: errors.New("EOF"),
}

fmt.Printf("something went wrong: %v", err)

What did you expect to see?

something went wrong: Get http://localhost/test/path: EOF

What did you see instead?

something went wrong: Get "http://localhost/test/path": EOF

The URL is now quoted.

This originates from PR #29384 and is mentioned here: stellar/go#2325 (comment)

The documentation in the release notes of go1.14 is missing for this change.

@ianlancetaylor ianlancetaylor added this to the Go1.14.1 milestone Mar 3, 2020
@ianlancetaylor ianlancetaylor added help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Mar 3, 2020
@ianlancetaylor
Copy link
Contributor

See #29261 and https://golang.org/cl/185117.

CC @stefanb @mvdan @dsnet

We even considered reverting the change (in https://golang.org/cl/195978), but we still forgot to add a release note. Sigh.

@ianlancetaylor ianlancetaylor changed the title Missing documentation of quoting the URL of url.Errors in go1.14 release notes doc: missing documentation of quoting the URL of url.Errors in go1.14 release notes Mar 3, 2020
@dmitshur dmitshur modified the milestones: Go1.14.1, Go1.15 Mar 3, 2020
@dmitshur
Copy link
Contributor

dmitshur commented Mar 3, 2020

This needs to be fixed on master branch first, then it can be backported to Go 1.14.x. I've changed the milestone of this issue to 1.15, and will open a backport issue for 1.14.x.

@gopherbot Please open a backport issue for Go 1.14; this is a documentation fix.

@gopherbot
Copy link

Backport issue(s) opened: #37630 (for 1.14).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

@gopherbot
Copy link

Change https://golang.org/cl/222037 mentions this issue: doc/go1.14: document that unparsable URL in net/url/Error is now quoted

@gopherbot
Copy link

Change https://golang.org/cl/222317 mentions this issue: [release-branch.go1.14] doc/go1.14: document that unparsable URL in net/url.Error is now quoted

gopherbot pushed a commit that referenced this issue Mar 6, 2020
…et/url.Error is now quoted

Updates #37614
Updates #36878
Updates #29384
Fixes #37630

Change-Id: I63dad8b554353197ae0f29fa2a84f17bffa58557
GitHub-Last-Rev: 5297df3
GitHub-Pull-Request: #37661
Reviewed-on: https://go-review.googlesource.com/c/go/+/222037
Reviewed-by: Ian Lance Taylor <iant@golang.org>
(cherry picked from commit 2b0f481)
Reviewed-on: https://go-review.googlesource.com/c/go/+/222317
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
@mvdan
Copy link
Member

mvdan commented Mar 8, 2020

I'm the one who approved and merged the change, so it's my bad for not remembering about the release notes. Apologies.

@dmitshur
Copy link
Contributor

dmitshur commented Mar 9, 2020

No problem @mvdan. Thank you and @stefanb for contributing this improvement to Go 1.14 (FWIW, I've already benefitted from it at least once). There are many changes that go into a major Go release, and it's not always easy to know what is significant enough to be worth adding to the release notes. It is very helpful for the change author to assist with the relevant release note entry, if any, but writing release notes is an effort many people can contribute to.

To enumerate some tools that can be used in the future to help ensure a change isn't missed for release notes:

  • Adding a RELNOTE marker in the commit message, or any of the comments in the CL.
  • Adding an entry to release notes in the same CL that a change (we weren't doing this much in the past, but it seems like a good practice, and so we're encouraging it now).

Edit: I've also opened #37751 about improving relnote to hopefully catch similar situations in the future.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants