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

workflows: add common Go linter, fix #21 #34

Merged
merged 1 commit into from
Sep 9, 2024
Merged

Conversation

roman-khimov
Copy link
Member

This tries to unify golangci-lint settings we use across ~20 Go repositories and simplify maintenance. In general, we have two sources of golangci.yml files copied across different repositories: neofs-node and neo-go. They differ:

  • neofs-node doesn't care about tests, neo-go does
  • neofs-node has additional settings for gofmt/gomodguard/revive
  • neo-go has additional settings for issues with exclusions
  • neo-go doesn't use exhaustive
  • neofs-node doesn't use bodyclose/contextcheck/decorder/errorlint

The golangci.yml added here:

  • cares about tests
  • uses additional rules from neofs-node, they're useful
  • drops neo-go settings from the 'issues' section, they seem to be useless
  • enables all linters from both worlds

Which practically means that it causes some pain below the spine for just about any repository. But likely those can be fixed.

Most probably there are errors in this PR, but we can't check it untill it's merged.

@smallhive
Copy link

Unified config is good. Moreover each repo can tune it if required

Copy link
Member

@AnnaShaleva AnnaShaleva left a comment

Choose a reason for hiding this comment

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

drops neo-go settings from the 'issues' section, they seem to be useless

Why they are useless, are they applied by default? EXC0002 # should have a comment seems useful for me.

.github/workflows/go-linter.yml Outdated Show resolved Hide resolved
@roman-khimov
Copy link
Member Author

Why they are useless, are they applied by default? EXC0002 # should have a comment seems useful for me.

I've not noticed a lot of difference in previous runs, but let me check specific EXC* again, things may have changed over time.

@roman-khimov roman-khimov changed the title workflows: add common Go linter, fix #21 WIP: workflows: add common Go linter, fix #21 Sep 9, 2024
This tries to unify golangci-lint settings we use across ~20 Go repositories
and simplify maintenance. In general, we have two sources of golangci.yml
files copied across different repositories: neofs-node and neo-go. They differ:
 * neofs-node doesn't care about tests, neo-go does
 * neofs-node has additional settings for gofmt/gomodguard/revive
 * neo-go has additional settings for issues with exclusions
 * neo-go doesn't use exhaustive
 * neofs-node doesn't use bodyclose/contextcheck/decorder/errorlint

The golangci.yml added here:
 * cares about tests
 * uses additional rules from neofs-node, they're useful
 * drops exclusions made by neo-go settings, but keeps includes
 * enables all linters from both worlds

Which practically means that it causes some pain below the spine for just about
any repository. But likely those can be fixed.

Signed-off-by: Roman Khimov <[email protected]>
@roman-khimov
Copy link
Member Author

@AnnaShaleva, I've tried to test it in nspcc-dev/neo-bench#184, but it can't properly work because of https://github.com/orgs/community/discussions/31054. At the same time I know now it will work after the merge. Also, includes are back.

@roman-khimov roman-khimov changed the title WIP: workflows: add common Go linter, fix #21 workflows: add common Go linter, fix #21 Sep 9, 2024
@roman-khimov roman-khimov merged commit 36dae73 into master Sep 9, 2024
1 check passed
@roman-khimov roman-khimov deleted the add-linter branch September 9, 2024 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants