Skip to content

Conversation

@rrama
Copy link
Contributor

@rrama rrama commented Sep 8, 2025

Description

Migrate stylecheck, gosimple, and varcheck into staticcheck (they are now all combined).
Add unused linter with exported-fields-are-used: false, this is to replace varcheck's exported-fields: true.
Enable staticcheck explicitly, it was previously enabled, but was not listed.
Remove deprecated EXC#### exclusion rules.
Disable package comment rules (ST1000).
Disable naming convention rules (ST1003).
Move the timeout to command-line arguments (v2 discourages timeouts in the config).
Re-enable stuttering checks.
Fix any new lint errors.
Plus make pact install fails fail the make target.

Notes for the reviewer

I would recommend comparing .golangci.yaml with whitespace ignored.
Migration guide: https://golangci-lint.run/docs/product/migration-guide/

Checklist

  • Tests added and all succeed
  • None added.
  • Linted
  • README.md updated, if user-facing
  • N/A

🚨After having merged, please update the snyk-ls and CLI go.mod to pull in latest client.

Migrate stylecheck, gosimple, and varcheck into staticcheck (they are now all combined).
Add unused linter with `exported-fields-are-used: false`,
this is to replace varcheck's `exported-fields: true`.
Enable staticcheck explicitly, it was previously enabled, but was not listed.
Remove deprecated `EXC####` exclusion rules.
Disable package comment rules (ST1000).
Disable naming convention rules (ST1003).
Move the timeout to command-line arguments (v2 discourages timeouts in the config).
Re-enable stuttering checks.
Fix any new lint errors.
@snyk-io
Copy link

snyk-io bot commented Sep 8, 2025

🎉 Snyk checks have passed. No issues have been found so far.

security/snyk check is complete. No issues have been found. (View Details)

license/snyk check is complete. No issues have been found. (View Details)

code/snyk check is complete. No issues have been found. (View Details)

@rrama rrama marked this pull request as ready for review September 11, 2025 10:13
@rrama rrama requested review from a team as code owners September 11, 2025 10:13
Copy link
Contributor

@acke acke left a comment

Choose a reason for hiding this comment

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

LGTM

- unconvert
# TODO(unparam): revisit
#- unparam
- unused
Copy link
Contributor

Choose a reason for hiding this comment

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

You can safely remove the - unused line from your linters.enable section because it's already covered by staticcheck with checks: ['all'].

Unless you really want it to be explicit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Man, I really dislike these linters being enabled without you actually realising it... I got confused multiple times during this task about why I was seeing lint rules for things I had removed...
I don't know if it is better to be explicit or not, I will let you decide.

Copy link
Contributor

Choose a reason for hiding this comment

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

These linters do not need to be listed.

Linters you can safely remove:

  • unused (line 150) - Already covered by staticcheck
  • gosimple - Not explicitly listed, but if it were, it would be redundant
  • stylecheck - Not explicitly listed, but if it were, it would be redundant

All other linters in your configuration provide unique functionality not covered by staticcheck.

Id think either list all linters that can be listed (that are in use), or don't list linters that are enabled by staticcheck already?

Copy link
Contributor Author

@rrama rrama Sep 12, 2025

Choose a reason for hiding this comment

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

I was going to remove it and then I realised it feels weird to have unused listed in the settings, but not in the list of enabled linters. To someone who doesn't know better, this would look like a mistake.
I could add a comment on line 77 that it is enabled by staticcheck, would that be better than just listing it here even though it is redundant?

Previously a redownload would happen after the contents of the directory changed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants