Enable `unused` linter
The following discussion from !1956 (merged) should be addressed:
-
@steveazz started a discussion: (+2 comments) I've done some testing on this case, and I believe this is all caused by
unused
and notgosimple
ordeadcode
.unused
,deadcode
, andgosimple
As noted by you this doesn't work at all, and we can see this in https://gitlab.com/gitlab-org/gitlab-runner/-/jobs/508358301, if we run
top
on the machine that is running the job we can see that it's struggling:I've also tried with limiting memory usage but I still had no luck: https://gitlab.com/gitlab-org/gitlab-runner/-/jobs/508372944
gosimple
I decided to see what happens if we split out the
deadcode
andunused
into their own jobs whilst addinggosimpile
to run with the other linters https://gitlab.com/gitlab-org/gitlab-runner/pipelines/135317480 both the main one anddeadcode
seem to run successful! Howeverunused
still times out.gosimple
, anddeadcode
I've then added
deadcode
to the main job again to test out if we have any performance problems with this in https://gitlab.com/gitlab-org/gitlab-runner/-/jobs/508494772 and it seems to work as expected, whilst unused still killed the machine.If we ssh inside of the machine that is running the code_quality job we can see that memory consumption is under control:
With this information I think we should do the following:
- Add
deadcode
andgosimpile
inside of.golangci.yml
to be enabled by default - Remove the
ADDITIONAL_LINTERS
as at the moment it seems strange that we have a different between local execution and CI execution in CI without knowing about the memory problems (I know we are commenting it but it's still not obvious to the user unless they see it). Having two different set of linters running locally and on CI can cause problems in the future of "It works on my machine, why doesn't it work on CI". Having running the same linters locally and inside of CI also represents a single source of truth being our.golanci.yml
. Also people are less likely to runmake lint
on there machine but rather leave it up to the CI to deal with it. -
unused
is still quite a fairly useful linter so ideally, we should enable it in the future. We should open a separate issue to understand, investigate and fix the usage ofunused
inside of our CI. There are few things we can investigate such as playing withGOGC
andnew: true
- Add