🐛 Ignore ireturn, and varnamelen linters

[cardil]

Changes

• 🐛 Ignore ireturn, and varnamelen linters

/kind bug

Fixes #98

Justification

I did a closer look at those two new rules that came with the latest release of golangci-lint: ireturn and varnamelen, and I believe it's not worth pursuing compliance with them. Detailed evaluation below.

ireturn evaluation

The ireturn linter ensures you don't return interfaces, but rather a concrete type. In general, that's good practice when the struct is public - the caller might be interested in struct fields, not only the behavior (interface). But, ireturn fires also in case when the returned type is private - linter even advise such change in its README doc. Doing that violates another linter - revive, and it's cumbersome to caller, not giving him any benefits from it.

There are other cases that seem reasonable to skip, like:

• factory methods #21,
• chain of responsibility pattern #22
• private structs with single interface #31

Also, worth citing Rob Pike on this go-proverbs/go-proverbs.github.io#37 (comment)

varnamelen evaluation

The varnamelen forces use of names size 3+ for most of the variables. It applies to all places where the function is longer than 5 lines, so almost all places. That makes it really hard to use, as Go code tends to have short packages, like git, file, metadata, plugin, and so 3+ length variable names tend to interfere a lot with those packages.

I've experimented a bit with different settings:

# The longest distance, in source lines, that is being considered a "small scope." (defaults to 5)
# Variables used in at most this many lines will be ignored.
max-distance: 12
# The minimum length of a variable's name that is considered "long." (defaults to 3)
# Variable names that are at least this long will be ignored.
min-name-length: 2

And still, I got 10 places that are not compliant, in such small project. Manually reviewing them, I don't see a huge benefit of changing those names. It doesn't improve readability IMHO.

[cardil]

/cc @dsimansk

[codecov]

Codecov Report

Merging #102 (4608dd5) into main (43ff059) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #102   +/-   ##
=======================================
  Coverage   69.92%   69.92%           
=======================================
  Files          31       31           
  Lines         828      828           
=======================================
  Hits          579      579           
  Misses        192      192           
  Partials       57       57           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 43ff059...4608dd5. Read the comment docs.

[cardil]

This change is taken from #99

[dsimansk]

I hope that won't break update actions 🙈

[cardil]

I think it won’t.

[dsimansk]

/approve
/lgtm

[knative-prow-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cardil, dsimansk

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [cardil,dsimansk]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[cardil]

/cherrypick release-0.26
/cherrypick release-1.0

[knative-prow-robot]

@cardil: new pull request created: #109

In response to this:

/cherrypick release-0.26
/cherrypick release-1.0

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.

[cardil]

/cherrypick release-1.0

[knative-prow-robot]

@cardil: new pull request created: #110

In response to this:

/cherrypick release-1.0

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.

[cardil]

/cherrypick release-0.25

[knative-prow-robot]

@cardil: new pull request created: #111

In response to this:

/cherrypick release-0.25

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.