From c8bed6a04345da5c868d120d9b45d82d84a2dbf0 Mon Sep 17 00:00:00 2001 From: Masayuki Morita Date: Thu, 9 Sep 2021 10:20:24 +0900 Subject: [PATCH 1/2] Use revive instead of golint The golint was officially deprecated. https://github.com/golang/go/issues/38968 We have been using golint via golangci-lint and golangci-lint now recommends using revive instead of golint. https://golangci-lint.run/usage/linters/ > golint: The repository of the linter has been archived by the owner. > Replaced by revive. The revive is a drop-in replacement of golint. I think switching to revive is a reasonable choice to check the same rules for free. https://github.com/mgechev/revive --- .github/workflows/lint.yml | 12 ++++++------ .golangci.yml | 2 +- testdrive/utils.go | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 2cf420a1ef..29a5aab838 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -14,18 +14,18 @@ jobs: github_token: ${{ secrets.GITHUB_TOKEN }} tool_name: golangci-lint - # Use golint via golangci-lint binary with "warning" level. - golint: - name: runner / golint + # Use revive via golangci-lint binary with "warning" level. + revive: + name: runner / revive runs-on: ubuntu-latest steps: - name: Check out code into the Go module directory uses: actions/checkout@v1 - - name: golint + - name: revive uses: reviewdog/action-golangci-lint@v2 with: - golangci_lint_flags: "--disable-all -E golint" - tool_name: golint # Change reporter name. + golangci_lint_flags: "--disable-all -E revive" + tool_name: revive # Change reporter name. level: warning # GitHub Status Check won't become failure with this level. # You can add more and more supported linters with different config. diff --git a/.golangci.yml b/.golangci.yml index 0630766f93..e403447ec5 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -6,7 +6,7 @@ linters: # We don't use goconst because it gives false positives in the tests. # - goconst - gofmt - - golint + - revive - gosec - gosimple - ineffassign diff --git a/testdrive/utils.go b/testdrive/utils.go index 77645e45b6..1452951a20 100644 --- a/testdrive/utils.go +++ b/testdrive/utils.go @@ -214,7 +214,7 @@ func execAndWaitForStderr(wg *sync.WaitGroup, stderrMatch *regexp.Regexp, timeou cancel() // We still need to wait for the command to finish. command.Wait() // nolint: errcheck - return cancel, errChan, fmt.Errorf("timeout, logs:\n%s\n", log) // nolint: staticcheck, golint + return cancel, errChan, fmt.Errorf("timeout, logs:\n%s\n", log) // nolint: staticcheck, revive } // Increment the wait group so callers can wait for the command to finish. From b0fedf79a28246fca2828eacfee19cfdea3c4049 Mon Sep 17 00:00:00 2001 From: Masayuki Morita Date: Fri, 10 Sep 2021 18:45:43 +0900 Subject: [PATCH 2/2] Fix lint errors reported by revive The revive is almost the same as golint, but there seems to be a bit different results. https://app.circleci.com/pipelines/github/runatlantis/atlantis/1519/workflows/75fd7da5-1532-4ed7-ac8a-4884902818a4/jobs/8314 https://github.com/runatlantis/atlantis/runs/3565887247?check_suite_focus=true --- server/core/locking/locking_test.go | 2 +- server/core/runtime/policy/conftest_client.go | 4 ++-- server/events/pre_workflow_hooks_command_runner_test.go | 6 +++--- server/events/yaml/valid/global_cfg.go | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/server/core/locking/locking_test.go b/server/core/locking/locking_test.go index fd8ce1e328..316b770ba5 100644 --- a/server/core/locking/locking_test.go +++ b/server/core/locking/locking_test.go @@ -178,7 +178,7 @@ func TestGetLock_NoOpLocker(t *testing.T) { l := locking.NewNoOpLocker() lock, err := l.GetLock("owner/repo/path/workspace") Ok(t, err) - var expected *models.ProjectLock = nil + var expected *models.ProjectLock Equals(t, expected, lock) } diff --git a/server/core/runtime/policy/conftest_client.go b/server/core/runtime/policy/conftest_client.go index 8adfdc99c4..620361f3c7 100644 --- a/server/core/runtime/policy/conftest_client.go +++ b/server/core/runtime/policy/conftest_client.go @@ -94,7 +94,7 @@ func (p *SourceResolverProxy) Resolve(policySet valid.PolicySet) (string, error) case valid.LocalPolicySet: return p.localSourceResolver.Resolve(policySet) default: - return "", errors.New(fmt.Sprintf("unable to resolve policy set source %s", source)) + return "", fmt.Errorf("unable to resolve policy set source %s", source) } } @@ -237,7 +237,7 @@ func getDefaultVersion() (*version.Version, error) { defaultVersion, exists := os.LookupEnv(DefaultConftestVersionEnvKey) if !exists { - return nil, errors.New(fmt.Sprintf("%s not set.", DefaultConftestVersionEnvKey)) + return nil, fmt.Errorf("%s not set", DefaultConftestVersionEnvKey) } wrappedVersion, err := version.NewVersion(defaultVersion) diff --git a/server/events/pre_workflow_hooks_command_runner_test.go b/server/events/pre_workflow_hooks_command_runner_test.go index e384a52353..3476e8446b 100644 --- a/server/events/pre_workflow_hooks_command_runner_test.go +++ b/server/events/pre_workflow_hooks_command_runner_test.go @@ -74,7 +74,7 @@ func TestRunPreHooks_Clone(t *testing.T) { t.Run("success hooks in cfg", func(t *testing.T) { preWorkflowHooksSetup(t) - var unlockCalled *bool = newBool(false) + var unlockCalled = newBool(false) unlockFn := func() { unlockCalled = newBool(true) } @@ -159,7 +159,7 @@ func TestRunPreHooks_Clone(t *testing.T) { t.Run("error cloning", func(t *testing.T) { preWorkflowHooksSetup(t) - var unlockCalled *bool = newBool(false) + var unlockCalled = newBool(false) unlockFn := func() { unlockCalled = newBool(true) } @@ -191,7 +191,7 @@ func TestRunPreHooks_Clone(t *testing.T) { t.Run("error running pre hook", func(t *testing.T) { preWorkflowHooksSetup(t) - var unlockCalled *bool = newBool(false) + var unlockCalled = newBool(false) unlockFn := func() { unlockCalled = newBool(true) } diff --git a/server/events/yaml/valid/global_cfg.go b/server/events/yaml/valid/global_cfg.go index 81068d11ab..6a630a8ca8 100644 --- a/server/events/yaml/valid/global_cfg.go +++ b/server/events/yaml/valid/global_cfg.go @@ -27,7 +27,7 @@ const DeleteSourceBranchOnMergeKey = "delete_source_branch_on_merge" // TODO: Make this more customizable, not everyone wants this rigid workflow // maybe something along the lines of defining overridable/non-overrideable apply // requirements in the config and removing the flag to enable policy checking. -var NonOverrideableApplyReqs []string = []string{PoliciesPassedApplyReq} +var NonOverrideableApplyReqs = []string{PoliciesPassedApplyReq} // GlobalCfg is the final parsed version of server-side repo config. type GlobalCfg struct {