Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configuration File .golangci.yml for golangci-lint Runs #1921

Merged
merged 4 commits into from
Mar 14, 2023

Conversation

Bhargav-InfraCloud
Copy link
Contributor

@Bhargav-InfraCloud Bhargav-InfraCloud commented Feb 21, 2023

Change Overview

Configuration file .golangci.yml for golangci-lint runs as explained in #1761.

Changes

  1. Linter: Run 7 default linters (errcheck, gosimple, govet, ineffassign, staticcheck, typecheck & unused) from .golangci.yaml.
  2. Linter: Run linter gofmt from .golangci.yaml.
  3. Linter: Run custom linters (maligned, whitespace, gocognit & unparam) from .golangci.yaml.
  4. Linter: Remove linter maligned due to following reasons:
    • Linter maligned is archived by owner and deprecated from golangci-lint.
    • maligned (or it's replacement: govet's fieldalignment) recommends >50 struct's field alignment. Too many changes for much less value.
    • Struct fields might be ordered per their similarities. Rearranging for saving few bytes is not reasonable.

Pull request type

Please check the type of change your PR introduces:

  • 🚧 Work in Progress
  • 🌈 Refactoring (no functional changes, no api changes)
  • 🐹 Trivial/Minor
  • 🐛 Bugfix
  • 🌻 Feature
  • 🗺️ Documentation
  • 🤖 Test

Issues

Test Plan

  • 💪 Manual
  • ⚡ Unit test
  • 💚 E2E

Run the command:

make golint

It should pass as it was before. Output similar to:

make[1]: Entering directory '/home/bhargav-infracloud/Workspace/go/src/github.com/Bhargav-InfraCloud/kanister'
running CMD in the containerized build environment
Running golangci-lint from config file: .golangci.yml
PASS

PASS

make[1]: Leaving directory '/home/bhargav-infracloud/Workspace/go/src/github.com/Bhargav-InfraCloud/kanister'

To Maintainers

The gofmt and govet are ran in make golint as well as the following scripts:
1. build/test.sh
2. examples/aws-rds/postgresql/pgtest/build/test.sh

Is there any particular reason for this or can these be eliminated from (or) replaced in scripts? This is discussed in the comments.

Comment on lines +29 to +31
- text: "`ctx` is unused" # Context might not be in use in places, but for consistency, we pass it.
linters:
- unparam
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want to address this. Can be addressed separately

Copy link
Contributor

@PrasadG193 PrasadG193 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@PrasadG193
Copy link
Contributor

Is there any particular reason for this or can these be eliminated from (or) replaced in scripts?

@Bhargav-InfraCloud examples/aws-rds/postgresql/pgtest/ is a sample app that users can clone and try. Considering it as a separate project, we have kept its build scripts in its directory.

@Bhargav-InfraCloud
Copy link
Contributor Author

@Bhargav-InfraCloud examples/aws-rds/postgresql/pgtest/ is a sample app that users can clone and try. Considering it as a separate project, we have kept its build scripts in its directory.

Makes sense. Thanks for letting me know and the approval.

@PrasadG193
Copy link
Contributor

@Bhargav-InfraCloud could you please add attach the output of make golint and update the test plan?

Move the default linter's config to .golangci.yaml file.
The 7 default linters: errcheck, gosimple, govet,
ineffassign, staticcheck, typecheck & unused.

Signed-off-by: Bhargav Ravuri <bhargav.ravuri@infracloud.io>
Move gofmt linter run to .golangci.yaml file.

Signed-off-by: Bhargav Ravuri <bhargav.ravuri@infracloud.io>
Move custom linters to .golangci.yaml file.
4 custom linters: maligned, whitespace, gocognit & unparam

Signed-off-by: Bhargav Ravuri <bhargav.ravuri@infracloud.io>
Reasons:
- Linter maligned is archived by owner and deprecated
  from golangci-lint.
- maligned (or it's replacement: govet's fieldalignment)
  recommends >50 struct's field alignment. Too many changes
  for much less value.
- Struct fields might be ordered per their similarities.
  Rearranging for saving few bytes is not reasonable.

Signed-off-by: Bhargav Ravuri <bhargav.ravuri@infracloud.io>
@Bhargav-InfraCloud
Copy link
Contributor Author

could you please add attach the output of make golint and update the test plan?

@PrasadG193 My bad I missed it. I've updated the desc now and rebased the code with the latest on master. Please check.

@mergify mergify bot merged commit 556c64e into kanisterio:master Mar 14, 2023
@Bhargav-InfraCloud Bhargav-InfraCloud deleted the golangci-lint branch March 14, 2023 17:36
@Bhargav-InfraCloud
Copy link
Contributor Author

Thanks @PrasadG193 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Linter] Configure .golangci.yml in place of golint.sh and add additional linters
2 participants