Instead of enabling all linters, and when disabling some of them because
we don't like them, switch to list of explicitly enabled linters. The
upside of this is we can easily upgrade golangci-lint (i.e. any new
linters won't break our CI). The downside is, we need to explicitly
enable extra linters.
To me, this is better from the maintainability perspective.
NOTE this commit does not change the configuration in any way, in other
words, the list of linters being run is the same as before. The next
commit will address this.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
- exportloopref is deprecated and deactivated so it should be removed
from the disable list.
- tenv is deprecated and was replaced by usetesting
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
- fix issues found by recvcheck
- skip k8s files from recvcheck
- remove two removed linters gomnd and execinquery
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Many dependencies started using go 1.22 which means we have to follow in
order to update.
Disable the now depracted exportloopref linter as it was replaced by
copyloopvar as go fixed the loop copy problem in 1.22[1]
Another new chnage in go 1.22 is the for loop syntax over ints, the
intrange linter chacks for this but there a lot of loops that have to be
converted so I didn't do it here and disable th elinter for now, th eold
syntax is still fine.
[1] https://go.dev/blog/loopvar-preview
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Remove deactivated linters from the config as they will be removed in
the future and thorw warnings, all of them were disabled already anyway
so this is no functional change.
Second, fix one new lint warning for fmt.Scanln() error checking.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
disable linters:
- mnd (magic number checker): it seems to complain about almost
anything, while there might be a few valid findings most of them are
useless
- canonicalheader: HTTP header syntax, most of the header are given by
docker and are stable so we cannot change them anyway
- execinquery: it has been deprecated
goerr113 was renamed to err113
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Disable the `perfsprint` and `typecheck` linters as they are too noisy.
There are some good issues worthy of fixing but it's too time consuming
at the moment. Hence, let's unblock the PR.
Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
The new linter testifylint reports way to many issues, although they
look like correct findings it is only a small expected vs actual swap
which should not effect the tests other than a (maybe )confusing
error message.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
But disable the `unused-parameter` linter as there are just too many
reports that I could handle. Also allow unused nolintlint reports.
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
It turns out, after iterating over rows, we need to check for errors. It
also turns out that we did not do that at all.
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
When golangci-lint it will only report 3 errors fromt he same linter by
default. This is annoying when a new linter is added and you think only
3 three errors lets fix it real quick only to notice when you rerun it
there again new 3 errors and so on.
In CI and local I want to see all issues at once so I can fix them and
know how much work it is before starting to fix them.
With `max-issues-per-linter: 0` and `max-same-issues: 0` it will show
us all errors because 0 means unlimted. By default it will only show 50
per linter and 3 from the same issue.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
The new version contains the ginkgolinter, which makes sure the
assertions are more helpful.
Also replace the deprecated os.SEEK_END with io.SeekEnd.
There is also a new `musttag` linter which checks if struct that are
un/marshalled all have json tags. This results in many warnings so I
disabled the check for now. We can reenable it if we think it is worth
it but for now it way to much work to fix all report problems.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Also fix a number of duplicate words. Yet disable the new `dupword`
linter as it displays too many false positives.
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
golangci-lint is throwing warnings on each run:
WARN [runner] The linter 'xxxxx' is deprecated (since v1.49.0)
due to: The owner seems to have abandoned the linter.
Replaced by unused.
...for xxxxx in deadcode, structcheck, varcheck. Add those three
to the deprecated-linter list, and remove any exceptions from
the code base.
Signed-off-by: Ed Santiago <santiago@redhat.com>
Motivated to have a working `make lint` on Fedora 37 (beta).
Most changes come from the new `gofmt` standards.
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
Update to the latest golangci-lint version. v1.46 added new linters.
I disabled nonamedreturns and exhaustruct since they enforce a certain
code style and using them would require big changes to the code base.
The nosprintfhostport is new and I fixed one problem in the tests. While
the test itself is fine because it uses ipv4 only the linter still looks
good because the sprintf use will fail for ipv6 addresses.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
The nolintlint linter does not deny the use of `//nolint`
Instead it allows us to enforce a common nolint style:
- force that a linter name must be specified
- do not add a space between `//` and `nolint`
- make sure nolint is only used when there is actually a problem
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
The errcheck linter makes sure that errors are always check and not
ignored by accident. It spotted a lot of unchecked errors, mostly in the
tests but also some real problem in the code.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
The linter ensures a common code style.
- use switch/case instead of else if
- use if instead of switch/case for single case statement
- add space between comment and text
- detect the use of defer with os.Exit()
- use short form var += "..." instead of var = var + "..."
- detect problems with append()
```
newSlice := append(orgSlice, val)
```
This could lead to nasty bugs because the orgSlice will be changed in
place if it has enough capacity too hold the new elements. Thus we
newSlice might not be a copy.
Of course most of the changes are just cosmetic and do not cause any
logic errors but I think it is a good idea to enforce a common style.
This should help maintainability.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
golint, scopelint and interfacer are deprecated. golint is replaced by
revive. This linter is better because it will also check for our error
style: `error strings should not be capitalized or end with punctuation or a newline`
scopelint is replaced by exportloopref (already endabled)
interfacer has no replacement but I do not think this linter is
important.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
We need to keep disabling the `nolintlint` linter to continue using
`nolint` directives in the code.
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
Disabling tagliatelle would mean disabling it for hundreds of fields
that cannot be changed due to K8s or Docker or backwards compat.
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
* supports Go 1.18
* disable a number of new linters
* fix minor stylecheck issues
[NO NEW TESTS NEEDED]
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
Cleanup the golangci.yml file and enable more linters.
`pkg/spec` and `iopodman.io` is history. The vendor directory
is excluded by default. The dependencies dir was listed twice.
Fix the reported problems in `pkg/specgen` because that was also
excluded by `pkg/spec`.
Enable the structcheck, typecheck, varcheck, deadcode and depguard
linters.
[NO TESTS NEEDED]
Signed-off-by: Paul Holzinger <paul.holzinger@web.de>
Reimplement CI-automation to remove accumulated technical-debt and
optimize workflow. The task-dependency graph designed goal was to
shorten it's depth and increase width (i.e. more parallelism). A
reduction in redundant building (and 3rd party module download) was
also realized by caching `$GOPATH` and `$GOCACHE` early on. This
cache is then reused in favor of a fresh clone of the repository
(when possible).
Note: The system tests typically execute MUCH faster than the
integration tests. However, contrary to a fail-fast/fail-early
principal, they are executed last. This was implemented due to
debug-ability related concerns/preferences of the primary
(golang-centric) project developers.
Signed-off-by: Chris Evich <cevich@redhat.com>
