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

Jump to bottom

doc: BuildNameToCertificate deprecated in go 1.14 not mentioned in the release notes #37626

Closed
coderste opened this issue Mar 3, 2020 · 14 comments
Closed

doc: BuildNameToCertificate deprecated in go 1.14 not mentioned in the release notes #37626

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

Comments

@coderste
Copy link

coderste 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/shi03/Library/Caches/go-build"
GOENV="/Users/shi03/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/shi03/go"
GOPRIVATE=""
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"
GOMOD="/Users/shi03/code/apis/gs/capi/go.mod"
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/dc/pxx2c5t503jdfy7c29qrlmp9nkvqm9/T/go-build781481956=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Looks like the BuildNameToCertificate function was deprecated in 1.14 but there's no mention of this on the release page.

What did you expect to see?

Release page mentioning the deprecated

What did you see instead?

No mention of the deprecation
@coderste coderste closed this as completed Mar 3, 2020
@coderste coderste reopened this Mar 3, 2020
@dmitshur
Copy link
Contributor

dmitshur commented Mar 3, 2020

The release notes are meant to provide a high level description of changes. Every single change that went into Go 1.14 cannot be included. I'm not sure if this is within scope of release notes, since it's easy to see that a function is deprecated by looking at package documentation.

I'll let package owners decide if anything should be done.

/cc @FiloSottile @katiehockman @ianlancetaylor

@dmitshur dmitshur changed the title BuildNameToCertificate deprecated in go 1.14 not mentioned in the release notes doc: BuildNameToCertificate deprecated in go 1.14 not mentioned in the release notes Mar 3, 2020
@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 3, 2020
@dmitshur dmitshur added NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Mar 3, 2020
@dmitshur dmitshur added this to the Go1.15 milestone Mar 3, 2020
@coderste
Copy link
Author

coderste commented Mar 4, 2020 •
edited

Ah fair enough I just thought it was missed rather than being done intentionally. In that case happy for this to be closed as resolved :)

@ianlancetaylor
Copy link
Contributor

ianlancetaylor commented Mar 4, 2020 •
edited

I certainly agree that not every change can be mentioned in the release notes, but I do tend to feel that all new functions should be documented and, correspondingly, that all decisions to deprecate a function should be documented.

@ianlancetaylor ianlancetaylor added help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Mar 4, 2020
@gopherbot gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Mar 4, 2020
@Sherlock-Holo
Copy link

make something deprecated is not common, a lot of people only read release notes to consider updating go version or not, then make a test. We invoke a lot of functions and if we need to read every function document to see if it is deprecated or not, it is a bad idea.

By the way, even I still use BuildNameToCertificate, go vet doesn't report it. If the Goland doesn't tell me this function is deprecated, maybe I use it even update to go 1.15

@FiloSottile
Copy link
Contributor

Happy to add it to the release notes.

@Sherlock-Holo When something is deprecated it does not stop working, and go vet does not fail due to use of deprecated identifiers, that's expected. Other static analysis tools like staticcheck do.

@FiloSottile
Copy link
Contributor

@dmitshur this will need backporting to show up on golang.org right?

@dmitshur
Copy link
Contributor

dmitshur commented Mar 23, 2020 •
edited

@FiloSottile Yes, to release-branch.go1.14, since the file to be modified is in the main Go repository.

@FiloSottile
Copy link
Contributor

@gopherbot please open a backport issue for Go 1.14, this is a documentation change that needs to go live on golang.org.

@gopherbot gopherbot mentioned this issue Mar 23, 2020
Closed
@gopherbot
Copy link

Backport issue(s) opened: #38030 (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.

@Sherlock-Holo
Copy link

@FiloSottile when something is deprecated, it is not a good idea for continuing using it, if it is not defective, why we deprecate it? IMO, the better idea is finding another way to replace it.

use tool likes staticcheck is a good idea, but writing it into release notes can let people know it is deprecated as soon as possible

@FiloSottile
Copy link
Contributor

There are a few reasons to deprecate something. In this case NameToCertificate is limited in its flexibility and fully replaced by better Certificates selection logic, so no new application should use it. Hence, deprecation. However, if an application is currently using it and it works for them, there is no urgency to switch away from it. We don't break applications unless strictly necessary.

I don't disagree about highlighting it in the release notes, though!

@gopherbot gopherbot mentioned this issue Mar 30, 2020
Closed
nickolaev pushed a commit to kumahq/kuma that referenced this issue May 9, 2020
fix(kuma-cp) remove deprecated functin call
02b1497 
Go 1.14 deprecates BuildNameToCertificate as described here
golang/go#37626

Fixing the linter exposes this issue and it's fixed here.

Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
nickolaev pushed a commit to kumahq/kuma that referenced this issue May 11, 2020
fix(kuma-cp) remove deprecated functin call
0d01369 
Go 1.14 deprecates BuildNameToCertificate as described here
golang/go#37626

Fixing the linter exposes this issue and it's fixed here.

Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
nickolaev pushed a commit to kumahq/kuma that referenced this issue May 11, 2020
chore(*) cleanup after upgrading to go 1.14 (#727)
b84f990 
* chore(*) bump gloangci-lint version

after e4d31d, the linter does not finish its job and complains  with
"WARN [runner] Can't run linter goanalysis_metalinter" and
"export data is newer version - update tool". This is described here
golangci/golangci-lint#893. The solution is to
use newer golangci-lint, which is done here.

* fix(kuma-cp) remove deprecated functin call

Go 1.14 deprecates BuildNameToCertificate as described here
golang/go#37626

Fixing the linter exposes this issue and it's fixed here.

* chore(*) cleanup dependencies `go mod tidy`

* chore(*) add more `-mod=mod` where needed

* fix(kuma-cp) fix failing linter after rebase to new k8s

Also Move the namespace creation to BeforeSuite section in k8s
store testing.

* chore(*) add tidy target for go mod maintenance

* chore(*) commit the result of the tidy

* fix(*) pushd/popd are not always available in a Makefile

Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
@ianlancetaylor
Copy link
Contributor

Is there anything left to do here?

@gopherbot
Copy link

Change https://golang.org/cl/240005 mentions this issue: doc/go1.14: crypto/tls.Config.NameToCertificate is deprecated

@gopherbot
Copy link

Change https://golang.org/cl/240011 mentions this issue: [release-branch.go1.14] doc/go1.14: crypto/tls.Config.NameToCertificate is deprecated

gopherbot pushed a commit that referenced this issue Jun 26, 2020
@ianlancetaylor
[release-branch.go1.14] doc/go1.14: crypto/tls.Config.NameToCertifica…
67d894e 
…te is deprecated

Also crypto/tls.Config.BuildNameToCertificate.

Note that this field and method were deprecated in the Go 1.14 release,
so this change is to the 1.14 release notes.

For #37626
Fixes #38030

Change-Id: If8549bc746f42a93f1903439e1b464b3e81e2c19
Reviewed-on: https://go-review.googlesource.com/c/go/+/240005
Reviewed-by: Filippo Valsorda <filippo@golang.org>
(cherry picked from commit 186e61f)
Reviewed-on: https://go-review.googlesource.com/c/go/+/240011
@golang golang locked and limited conversation to collaborators Jun 26, 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 help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.15
Development

No branches or pull requests
6 participants
@FiloSottile @dmitshur @ianlancetaylor @gopherbot @Sherlock-Holo @coderste