[Bug] Fix CI linter errors

[Olshansk]

Description

Summary generated by Reviewpad on 10 Jul 23 23:40 UTC

This pull request includes the following changes:

• Added a new file .github/workflows/golangci-lint.yml for golangci-lint workflow.
• Modified the .github/workflows/main.yml file by removing the golangci-lint job.
• Updated the .golangci.yml file to remove the typecheck linter.
• Updated the Makefile to add installation of helm and tilt as part of install_cli_deps task.
• Modified the app/client/cli/consensus.go file by adding an import statement.
• Modified the p2p/config/config.go file to return an error in the IsValid() method.
• Modified the p2p/raintree/router.go file by removing the unused import and variable.

Overall, this pull request adds a new golangci-lint workflow, updates some workflow files, makes changes to the Makefile, and modifies a few Go files.

Origin Document

I merged something unrelated to main and saw that the CI was broken

https://github.com/pokt-network/pocket/actions/runs/5468898070

Type of change

Please mark the relevant option(s):

• New feature, functionality or library
• Bug fix
• Code health or cleanup
• Major breaking change
• Documentation
• Other

List of changes

• Fix linter errors on main
• Make the linter run on branches as a separate workflow

Testing

• make develop_test; if any code changes were made
• make test_e2e on k8s LocalNet; if any code changes were made
• e2e-devnet-test passes tests on DevNet; if any code was changed
• Docker Compose LocalNet; if any major functionality was changed or introduced
• k8s LocalNet; if any infrastructure or configuration changes were made

[okdas]

Thanks @Olshansk! I opened a PR (#850) to fix this and had planned to return to it. However, I like your concept of employing a separate golangci-lint workflow.

The existing approach configures linter to run only against modified files, allowing some errors to slip through. If we constantly run the linter on the entire codebase (by setting only-new-issues to false), it'll be more effective. It also won't impact the CI times, as it runs in parallel without delaying the main build.

To avoid redundancy, let's remove the current linter step from the main pipeline and get rid of only-new-issues: true.

I can apply these changes to your branch if that's okay.

[Olshansk]

I can apply these changes to your branch if that's okay.

Sgtm, go ahead!

[okdas]

@Olshansk with linter now checking the whole codebase, it found more issues. I'll investigate them tomorrow.

[Olshansk]

@Olshansk with linter now checking the whole codebase, it found more issues. I'll investigate them tomorrow.

@okdas Have you had a chance to look into this?

[okdas]

@okdas Have you had a chance to look into this?

Yes, I'm working on this now. I wasn't able to reproduce locally, though, and have some ideas as of why this is happening.

[okdas]

@Olshansk Here's what's happening. The new CI job only pulls the source code, which doesn't include any code-generated stuff. As a result, we got a bunch of typecheck errors. I've added the necessary dependencies for that CI job, and it now comes back green.

I tried to remove the typecheck linter because it is redundant anyway (it does the same check the Go compiler performs during compilation), but there is no way to disable it via config, and some CLI voodoo is required.

At this point, I think it should be good to go (GTG). I've approved the PR; please merge if you are okay with my changes.