kubernetes: Fix golint failures
Anyone itching to understand the code and try fixing things in Kubernetes, please help fixing the lint failures
Step 1: Remove one or two packages (related packages) from the list in - https://github.com/kubernetes/kubernetes/blame/master/hack/.golint_failures (note: do not change generated files, the name of defaulting functions, or the name of conversion functions… packages failing linting due to issues in those files will need to be left alone)
Step 2: Run hack/verify-golint.sh and look at the logged errors
Step 3: Fix the code to ensure that hack/verify-golint.sh runs cleanly.
Step 4: Run hack/update-bazel.sh and hack/update-gofmt.sh to make sure we fix any problems with gofmt or bazel.
Step 5: File a PR with the change
Step 6: Go to step #1 😃
Please use the tracking spreadsheet here For everyone who is going to contribute,
- please append the packages you picked at the end.
- For existing contributor, please make sure this list is update to date
About this issue
- Original URL
- State: closed
- Created 6 years ago
- Reactions: 33
- Comments: 121 (99 by maintainers)
Commits related to this issue
- Modification: revise some errors about golint in some packages 1. pkg/client 2. staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing Related to: https://github.com/kubernetes/kubernetes... — committed to fengzixu/kubernetes by fengzixu 6 years ago
- Fix golint for pkg/probe This change adds comments to exported things and renames the tcp, http, and exec probe interfaces to just be Prober within their namespace. Issue #68026 — committed to jonfriesen/kubernetes by jonfriesen 6 years ago
- Merge pull request #68113 from fengzixu/master Fixes #68026: revise some errors about golint in some packages — committed to li-ang/kubernetes by k8s-ci-robot 6 years ago
- Modification: revise some errors about golint in some packages 1. pkg/client 2. staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing Related to: https://github.com/kubernetes/kubernetes... — committed to kubernetes/apiserver by fengzixu 6 years ago
- Fixes golint for test/list Fixes minor golint errors in the test/list tool. ref: #68026 — committed to jonfriesen/kubernetes by jonfriesen 6 years ago
- Fix golint errors in kube-apiserver This change exports type `CompletedServerRunOptions`, which is being used in return value for `Complete` function. Ref #68026 — committed to ifosch/kubernetes by ifosch 6 years ago
- Fix golint for pkg/probe This change adds comments to exported things and renames the tcp, http, and exec probe interfaces to just be Prober within their namespace. Issue #68026 — committed to vithati/kubernetes by jonfriesen 6 years ago
- Fixes golint for test/list Fixes minor golint errors in the test/list tool. ref: #68026 — committed to vithati/kubernetes by jonfriesen 6 years ago
- golint fixes in pkg/kubectl/cmd/annotate #68026 — committed to riaan53/kubernetes by riaan53 5 years ago
- Fix golint failures in cmd/cloud-controller-manager/app/apis/config/v1alpha1 Part of #68026. Adds comments to exported function. Renames SetDefaults_CloudControllerManagerConfiguration to SetDefault... — committed to guineveresaenger/kubernetes by deleted user 5 years ago
- Fix golint failures in pkg/util/taints Part of addressing #68026 — committed to markmc/kubernetes by markmc 5 years ago
- Fix golint issues in pkg/kubectl/cmd/annotate Issue #68026 — committed to palnabarun/kubernetes by palnabarun 5 years ago
- Fixes lint issues in pkg/cmd/kubectl/delete Issue #68026 — committed to palnabarun/kubernetes by palnabarun 5 years ago
After a couple years of seeing changes spawned from this issue, I don’t think this is a good use of contributors’ or reviewers’ time. golint is very opinionated about:
the govet and staticcheck verify jobs are much more useful since they catch correctness issues, but I would recommend closing this issue and dropping the golint checks from CI
golint doc itself discourages from using its output in an automated enforced way:
from https://github.com/golang/lint#purpose:
@yue9944882 +1 to slightly larger pulls with related packages. if allow too large then the reviewers will not be able to do a thorough review. we should spread the load among multiple reviewers. (if we allow a really large pull, we will end up with top level reviewers who will face the burden and the PR(s) will just stagnate)
@dims Created a spreadsheet with all the package that have on going PRs. Maybe you can add this to the summary.
we get 800-ish pkgs left to fix. my concern is that if we fix them package by package, then such pulls will be flooding the repo. can’t we possibly make it in larger pulls? the larger the better?
thanks everyone for getting us this far. We essentially gave up on the existing structure/system for golint: https://github.com/kubernetes/kubernetes/pull/98063
At some point we can look at adding golangci-lint, there’s a WIP in progress in https://github.com/kubernetes/kubernetes/pull/93656
/close
@guineveresaenger looks like the disconnect is because that problem does not show up in the version of
golintwe use in the CI.please see https://github.com/kubernetes/kubernetes/blob/master/hack/verify-golint.sh#L33-L34
I did indeed see the Google Sheet link (thank you @jonfriesen!) and hacked something together to take it a little further 😬
oh good point. we could put one pkg’s change into one commit, and merge all changes under one
OWNERarea into one pull so that it wont spread the load. wdyt? for each pull, we can have hundred-ish lines of change. but it will still be 30~50 pulls i guess? good news is that we can take our time to deal with it, at least before 1.12 😉Can someone please review and let me know if I’ve done this right? #97167
@formanojhr you need to remove package from https://github.com/kubernetes/kubernetes/blame/master/hack/.golint_failures first.
I’ve found a few others I’m actively looking at, you won’t get any argument from me.
On Sat, Oct 12, 2019, 10:38 AM Pamela Kelly notifications@github.com wrote:
Hi I am a new contributor here, I’d like to contribute and make changes to the following file/s. I have marked it as WIP in the Google Sheet. Can someone let me know if I have to do anything else before I start working on this PR?
hi @jlucktay thanks! there is a spreadsheet here that tracks the progress on fixing these issues, consider adding the packages you’re work on there so we can keep everything organized 👍 https://docs.google.com/spreadsheets/d/1VhU6zCk-vaAnbu_XkgsIq5I7jVnezUquWBMhcIN6AJM/edit#gid=0
hi @dims I think this issue was closed prematurely by #73255 (using fixes #issue in commit)
I’m looking at the pkg/registry/*, all those which are left. I expect to give an update on what all is fixed in the next 24-48 hours.
Sorry I misunderstood the issue
@war-turtle the failures are still present in https://github.com/kubernetes/kubernetes/blame/master/hack/.golint_failures
there are details in the first message on how to claim + solve issues 😃
@dims should we have a checklist as part of the issue summary saying what is in progress and what is done? Since this issue is getting long, this will make new contributors to pick up issues easier.