Skip to content

chore(*): update golangci-lint version from 1.16.0 to 1.20.1 #14

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

Merged
merged 1 commit into from
Nov 8, 2019
Merged

chore(*): update golangci-lint version from 1.16.0 to 1.20.1 #14

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,10 @@ build: build-local

# more info about `GOGC` env: https://github.com/golangci/golangci-lint#memory-usage-of-golangci-lint
lint: $(GOLANGCI_LINT)
@GOGC=5 $(GOLANGCI_LINT) run
@$(GOLANGCI_LINT) run

$(GOLANGCI_LINT):
curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b $(BIN_DIR) v1.16.0
curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b $(BIN_DIR) v1.20.1
Copy link
Member

Choose a reason for hiding this comment

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

是不是把 version 这玩意整成一个变量，放到前面去

Copy link
Member

Choose a reason for hiding this comment

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

只有一个地方用到，设不设变量无所谓

Copy link
Member

Choose a reason for hiding this comment

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

关键是保不齐下次又要改新版本。。。用变量传递的话，可以做到流水线直接传递。。就不需要改了

Copy link
Contributor

Choose a reason for hiding this comment

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

如果已经存在 golangci-lint 这个文件，更新版本号，$(GOLANGCI_LINT): 不会再执行的。想要用流水线控制下载的版本，那需要 golangci-lint 文件不存在，每次都拉。

Copy link
Member

Choose a reason for hiding this comment

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

流水线更新 golang:1.12.9-stretch 镜像 就可以了。

为了保证一致性，Makefile 还是得更新

Copy link

@hanxueluo hanxueluo Oct 25, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

我个人建议，不要从外部以变量的方式传递到Makefile里。 编译整套系统（包括里面的工具）最好是紧跟代码（分支）走。（虽然当前我们受限于流水线系统也没真正做到，但自己能控制的尽量自己控制住）
否则，外部lint工具做了更新扫出新的一顿错误，那历史分支就挂了。

Copy link

Choose a reason for hiding this comment

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

恩，对了，还有 golang 版本谁在关注呢？k8s 有更新 golang 版本 kubernetes/kubernetes#84064

不用跟得这么紧吧。。呼唤钊总 @iawia002

这个节奏还是你 @supereagle 定一下吧，proposal 里面写的是 follow 我们平台 k8 使用的 go 版本，但是我们平台的 k8 从来不升级小版本的。

  • 如果 follow 具体的 k8 小版本，比如现在的 1.14.2，那 go 基本是一个大的 cps 版本升级一次
  • 如果 follow 大版本，比如现在的 1.14.x，go 版本一个 cps 版本中估计要改好几次，因为 k8 基本在追最新的 go 版本（1.14.x --> 1.12.x），这样相当于我们也要一直追一个最新的 go 版本

Copy link
Member

Choose a reason for hiding this comment

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

我个人建议，不要从外部以变量的方式传递到Makefile里。

同意

这个节奏还是你 @supereagle 定一下吧，proposal 里面写的是 follow 我们平台 k8 使用的 go 版本，但是我们平台的 k8 从来不升级小版本的。

那就 follow k8s 的小版本吧，我们没有必要跟的这么紧，成本太高了

Copy link
Contributor

@Meoop Meoop Oct 25, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

很多更新的主要原因都是有涉及到安全的 bug 导致的，例如 1.12.9 ->1.12.10 修复的一个比较严重的 CVE-2019-16276（net/http 的 bug，在一些配置情况下可以绕过 k8s 身份认证）。

Copy link
Member

@supereagle supereagle Oct 29, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@ijumps 对于上面提到的安全问题，我们在 2.10 中打算升级 k8s 小版本吗？


test:
@go test -p $(CPUS) $$(go list ./... | grep -v /vendor | grep -v /test) -coverprofile=coverage.out
Expand Down