Bump golangci-lint to v1.20.0 #5572
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This version comes with significant memory improvements, making some adjustments for small memory footprint in the CI obsolete.
|
Welcome @corneliusweig! |
k8s-ci-robot
added
cncf-cla: yes
|
Hi @corneliusweig. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
k8s-ci-robot
added
the
size/XS
|
Can one of the admins verify this patch? |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: corneliusweig The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Travis tests have failedHey @corneliusweig, 1st Buildmake testTravisBuddy Request Identifier: 7f56a740-ea1c-11e9-93dd-b3a5547eb4f4 |
|
Thanks for making this update ! I see the lint already found some new lint issues that was ignored by the previous version |
medyagh
approved these changes
Oct 9, 2019
| # Limit number of default jobs, to avoid the CI builds running out of memory | ||
| GOLINT_JOBS ?= 4 | ||
| # see https://github.com/golangci/golangci-lint#memory-usage-of-golangci-lint | ||
| GOLINT_GOGC ?= 8 | ||
| GOLINT_GOGC ?= 100 |
There was a problem hiding this comment.
what is the reason for increasing 8 to 100 ?
There was a problem hiding this comment.
GOGC=100 is the default, so it is basically removing the increased garbage collection
There was a problem hiding this comment.
Yes, I restored the default, but I left the link to the docs in case that setting needs to be adjusted in the future.
medyagh
requested changes
Oct 9, 2019
|
When I run it locally (with 4 cpus/jobs), this lowers the memory usage from 4G to 1G: $ \time make lint GOLINT_VERSION=v1.18.0
./out/linters/golangci-lint-v1.18.0 run --deadline 4m --build-tags "integration container_image_ostree_stub containers_image_openpgp" --enable goimports,gocritic,golint,gocyclo,interfacer,misspell,nakedret,stylecheck,unconvert,unparam --exclude 'variable on range scope.*in function literal|ifElseChain' ./...
62.86user 3.91system 0:20.67elapsed 322%CPU (0avgtext+0avgdata 4165192maxresident)k
582344inputs+640outputs (665major+1436497minor)pagefaults 0swaps$ \time make lint GOLINT_VERSION=v1.20.0
./out/linters/golangci-lint-v1.20.0 run --deadline 4m --build-tags "integration container_image_ostree_stub containers_image_openpgp" --enable goimports,gocritic,golint,gocyclo,interfacer,misspell,nakedret,stylecheck,unconvert,unparam --exclude 'variable on range scope.*in function literal|ifElseChain' ./...
Flag --deadline has been deprecated, flag will be removed soon, please, use .golangci.yml config
cmd/minikube/cmd/start.go:129:2: var `enableUpdateNotification` is unused (unused)
enableUpdateNotification = true
^
pkg/minikube/extract/extract.go:397:47: `checkBinaryExpression` - `e` is unused (unparam)
func checkBinaryExpression(b *ast.BinaryExpr, e *state) string {
^
pkg/minikube/tunnel/tunnel_manager.go:125:43: (*Manager).cleanup - result 0 (*k8s.io/minikube/pkg/minikube/tunnel.Status) is never used (unparam)
func (mgr *Manager) cleanup(t controller) *Status {
^
Makefile:330: receptet för målet ”lint” misslyckades
make: *** [lint] Fel 1
Command exited with non-zero status 2
185.80user 7.64system 0:54.21elapsed 356%CPU (0avgtext+0avgdata 1504696maxresident)k
829872inputs+423928outputs (925major+2657303minor)pagefaults 0swapsIf that is not enough for the servers, you could try something like GOGC=20 ? https://github.com/golangci/golangci-lint#memory-usage-of-golangci-lint I think it still takes way to much time to run locally (1-3 minutes?), though. |
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
|
Apparently it now allocates 7.5G of RAM and 2 cores: And took 3 minutes (188 seconds) to run, # 8899.2 |
|
@corneliusweig - Thank you! Do you mind fixing the lint issues so that we don't end up with persistently failing tests? |
k8s-ci-robot
added
size/S
|
@tstromberg I fixed the new issues found by the linter. I'm pretty blown away that @afbjorklund Thanks for doing such thorough memory usage checking. As far as I understand, there's no action needed, right? |
Travis tests have failedHey @corneliusweig, 1st Buildmake testTravisBuddy Request Identifier: 360778e0-eacf-11e9-90a0-7556777797a6 |
corneliusweig
force-pushed
the
w/upgrade-linter
branch
from
7de8b77
to
822087e
Compare
Travis tests have failedHey @corneliusweig, 1st Buildmake testTravisBuddy Request Identifier: 1c282f40-ead5-11e9-90a0-7556777797a6 |
corneliusweig
force-pushed
the
w/upgrade-linter
branch
from
822087e
to
c00d8b3
Compare
k8s-ci-robot
added
size/M
Codecov Report
@@ Coverage Diff @@
## master #5572 +/- ##
==========================================
- Coverage 36.85% 36.31% -0.54%
==========================================
Files 102 102
Lines 7354 7521 +167
==========================================
+ Hits 2710 2731 +21
- Misses 4293 4438 +145
- Partials 351 352 +1
|
k8s-ci-robot
added
size/S
Travis tests have failedHey @corneliusweig, 1st Buildmake testTravisBuddy Request Identifier: 2564c070-eb7d-11e9-b736-db238f783907 |
Travis tests have failedHey @corneliusweig, 1st Buildmake testTravisBuddy Request Identifier: 3b1c71b0-eb7d-11e9-b736-db238f783907 |
Travis tests have failedHey @corneliusweig, 1st Buildmake testTravisBuddy Request Identifier: 71a54680-eb87-11e9-b736-db238f783907 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This version of golangci-lint comes with significant memory improvements, making some
adjustments for small memory footprint in the CI obsolete.
See golangci/golangci-lint#337