[Umbrella Issue] Code style and linting errors cleanup

[Adirio]

This issue will track cleanup duties related to code style and linting errors. Each will be described in different issues and implemented in corresponding PR. Feel free to suggest more changes that should be applied under this category as comments and they will be added to the list. Keep the discussion related to a specific topic in their corresponding issues.

• Error string formatting: Error string formatting #1177 Merged
• Unused exported functions removal: Unused exported functions removal #1178 Merged
• Unused parameters replacement by blank identifier: Unused parameters replacement by blank identifier #1179 Merged
• Explicit errors handling: Explicit errors handling #1180
• Package name as exported variable/type/function prefix removal: Package name as exported variable/type/function prefix removal #1181 Merged
• Unification of receiver names: Unification of receiver names #1182 Merged
• Imports ordering: Imports ordering #1183 Merged
• Redundant type removal: Redundant type removal #1188

/kind cleanup

[camilamacedo86]

HI @Adirio,

Could you please provide what linter tool you are using and its report with the errors for each kind added here? Also, IHMO the first step would be adding the linter in the CI then, add its PR here and after that start to work to solve the issues faced. When all be fine the first PR to add the linter in the CI should pass and be merged.

[Adirio]

My IDE has an integrated linter and I've started from there. Adding linter to the CI is not recommended as lint errors are just style recommendations, sometimes they may not be followed due to any other reason and if it gets added to the CI process it would get blocked.

[camilamacedo86]

My IDE has an integrated linter and I've started from there

What linter is your iDE using?

Adding linter to the CI is not recommended as lint errors are just style recommendations

Linters are NOT just about style recommendations. See: https://github.com/golangci/awesome-go-linters

sometimes they may not be followed due to any other reason and if it gets added to the CI process it would get blocked.

It has something that should not be merged and checked then, usually it should be done automatically by the CI in order to avoid it, otherwise, the same kind of issues/prs will still be required and the problem will be not solved.

A commonly used linter aggregator for go is : https://github.com/golangci/golangci-lint.

Also, it does not get too long to be executed which usually is the problem/reason for do not add linters in the CI and can be called used by a command line which makes easier its usage. I'd recommend you use this one since the project has the PR: #830

[Adirio]

Quoted from golang/lint:

The suggestions made by golint are exactly that: suggestions. Golint is not perfect, and has both false positives and false negatives. Do not treat its output as a gold standard. We will not be adding pragmas or other knobs to suppress specific warnings, so do not expect or require code to be completely "lint-free". In short, this tool is not, and will never be, trustworthy enough for its suggestions to be enforced automatically, for example as part of a build process. Golint makes suggestions for many of the mechanically checkable items listed in Effective Go and the CodeReviewComments wiki page.

I'm not sure if my IDE is using golint as it is integrated in it (JetBrains' Golang) but the officail lint project suggests not to include it in CI.

[camilamacedo86]

HI @Adirio,

I made more than one suggestion in my above comment. Then, I will try to clarify better my POV here. I hope that it can help you achieve your goal as well.

Regards the PR's:

IHMO is great to work on fixing linter issues to improve the quality of the code, but, is required make clear what linter and its results as argumentation of the PR's in order to allow the reviewer check if it as well. Note that no make sense we have many PR's just to do the same as, for example, replace the capital letters to lower ones in the errors/messages.

Also, how someone can understand the purpose of each task if has not the description over how to reproduce the same scenario and check if all is solved? How someone can collab with the tasks raised by you?

See also, that the README page has a badge for: https://goreportcard.com/report/sigs.k8s.io/kubebuilder. May you like to use it to illustrate the motivation for the PR's.

Regards add a specific linter in the CI

I'd suggest we try to use golangci-lint and adopted it in the CI. Following the reasons.

• Ensure that the same issues will not be introduced after again
• Make clear what has been checked and what is expected
• It is a common linter to be used
• It is easy to be called since allows we check it by a command line
• It is fast and will not make the PR's take to long to be checked.
• Also, because the project has already Golangci lint fixes for scaffold projects #830 to add it and fix the issues for the scaffold project and shows that it was accepted by the project maintainers.
• Note that it has annotations to ignore linter specific lines of code if be required and avoid the scenario point out by you which can be faced with GoLint. See https://github.com/golangci/golangci-lint#nolint

@DirectXMan12 and @droot. Could we adopt this one? Any objection about doing a PR with it in the CI for we are able to check its errors and the progress of this task which @Adirio's purpose seems to solve all possible linter issues in the project? So, when it be finished means that this PR should pass in the CI. WDYT ?

[Adirio]

I'm not going to answer the "linter in CI" topic as I think that is a higher level decission that should be taken by the project mantainers who you already mentioned. This may actually be a good place to discuss it unless any other mean is proposed.

About the other comments:

• They are project-wide PRs. All of them. As it is a small-ish project, some of the PRs are pretty small, but I inspected the whole kubebuilder code. The only part that may be skiped by the linter is the generated files, I'll try to have a look at them but that will require mostly manual inspection as they are strings and scaffolded files outside of expected paths (v1 testdata for example).
• I'll try to improve the description but the titles contain most of the information that it is provided by the linter. Will try to look for a bit longer descriptions.

[Adirio]

@camilamacedo86 I added the exact message and description from the linter in the four Issues that have a PR (and in their corresponding PRs too). Will do the same when I tackle the remaining ones.

[camilamacedo86]

Hi @Adirio,

How can we reproduce the issue and check the messages in order to verify your PRs? How can any contributor work on in the tasks whiteout the linter info?

from the above msg:

See also, that the README page has a badge for: https://goreportcard.com/report/sigs.k8s.io/kubebuilder. May you like to use it to illustrate the motivation for the PR's.

I hope that the above questions can clarify the points.

[Adirio]

As you may see in the specific issues most of the modifications are related to CodeReviewComments or Effective Go. Most linters should hint this errors. In my case, Im using the linter hints from my IDE, which is not open source nor free so I can't suggest to download it.
Note: Bugs require reproducibility, but this are cleanup-oriented issues and PRs, that's why I don't think that exact steps need to be provided. Don't missunderstand me, if I could provide them I would, but as I'm using a IDE that isn't free it would not be that useful.

[camilamacedo86]

Your IDE uses a specific linter. So, in order to clarify my suggestion would be you check what linter is and add in the comment. It would be enough.

[Adirio]

It is builtin in the IDE, it is not an external plugin. The code of the IDE is not open source. The only documentation provided is:

Inspections & quick-fixes

The IDE provides built-in inspections that check your code on the fly as you type it. When they find problematic code, they provide you with quick-fixes which you can apply simply by pressing Alt+Enter.

[camilamacedo86]

HI, could you please assign the issue to you in order to avoid other dev get it since you gave a PR already one for this one? Also, devs will usually filter for the unassigned ones to know what they can do to contribute with.

PS.: To do it just add a new comment with /assign @your user

[Adirio]

/assign