You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
We currently only run golangci-lint on certain packages in CI. We should run golangci-lint locally, fix all the warnings/errors and then start running golangci-lint on all packages in CI. The one thing to consider here is which linters should be enabled by default (or just use the default linters used by golangci-lint).
The text was updated successfully, but these errors were encountered:
Might want to prioritize because of bugs like #2589 (PR #2591). golangci-lint can catch that:
58:core/orchestrator.go:634:4: lostcancel: the cancel function is not used on all paths (possible context leak) (govet)
59- ctx, cancel := context.WithTimeout(context.Background(), transcodeLoopTimeout)
60- ^
61:core/orchestrator.go:639:5: lostcancel: this return statement may be reached without using the cancel var defined on line 634 (govet)
62- return
63- ^
There are about 200 linting warnings currently, I see a few options how to minimize efforts of enabling it:
disable linters which check for less severe errors, like errcheck, govet, unused, gosimple
only lint files changed in the commit, which still may be painful and could be combined with option 1
disable linters which check for less severe errors
In the case of #2589 we would have wanted govet to be enabled to flag the context issues right?
only lint files changed in the commit, which still may be painful and could be combined with option 1
A few alternatives I am wondering about:
Lint changed lines only
Configure the linter to emit certain less severe errors as just warnings so that CI doesn't fail for them and only fails for the more "interesting" errors
Then, a gradual approach might look like:
Lint files changed in the commit
If someone opens a PR that fails linting, first create a separate PR that fixes the linting issues in the relevant files
Merge the separate PR and then rebase the original PR which should pass linting
We currently only run golangci-lint on certain packages in CI. We should run golangci-lint locally, fix all the warnings/errors and then start running golangci-lint on all packages in CI. The one thing to consider here is which linters should be enabled by default (or just use the default linters used by golangci-lint).
The text was updated successfully, but these errors were encountered: