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

[chore] add a few additional linters #25060

Closed
wants to merge 2 commits into from

Conversation

@atoulme atoulme requested review from a team and bogdandrutu August 8, 2023 06:41
Copy link
Member

@songy23 songy23 left a comment

Choose a reason for hiding this comment

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

@atoulme
Copy link
Contributor Author

atoulme commented Aug 8, 2023

Yes, we still have quite a few PRs to merge. I might need to split this further.

@dmitryax
Copy link
Member

dmitryax commented Aug 8, 2023

@atoulme
Copy link
Contributor Author

atoulme commented Aug 8, 2023

https://golangci-lint.run/usage/linters/#decorder
Does it mean that a struct declaration has to be separated from its methods?

This one didn't give me any trouble, it looks like its defaults mostly make it check nothing. Might be worth to try to play with its settings a bit more.

It looks like by default all it does is making sure init functions are first in a file. I think we could also mandate constants and vars to be up to the top.

@dmitryax
Copy link
Member

dmitryax commented Aug 8, 2023

It looks like by default all it does is making sure init functions are first in a file. I think we could also mandate constants and vars to be up to the top.

I think the default behavior is enough

@dmitryax
Copy link
Member

https://golangci-lint.run/usage/linters/#prealloc

Let's discuss it in #24956 (comment)

I think it's already enough people said "no" to this linter, so we can remove it

@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Aug 26, 2023
@atoulme atoulme removed the Stale label Aug 28, 2023
codeboten pushed a commit that referenced this pull request Sep 7, 2023
See #25060 for original request.

This adds the decorder linter with default configuration, just ensuring
init blocks are at the top of the file for now.
@atoulme
Copy link
Contributor Author

atoulme commented Sep 8, 2023

Closing as I have split this work in smaller PRs.

@atoulme atoulme closed this Sep 8, 2023
codeboten pushed a commit that referenced this pull request Sep 8, 2023
See
#25060
for original request.

This adds the reassign linter, checking no package variables are
reassigned.
codeboten pushed a commit that referenced this pull request Sep 8, 2023
See
#25060
for original request.

This adds the wastedassign linter with default configuration. It ensures
no assignmnent is made to a variable and then not used.
codeboten pushed a commit that referenced this pull request Sep 14, 2023
See #25060 for original request.

This adds the predeclared linter with an explicit carveout to ignore use
of the `copy` reserved word as it is used by a package.

Co-authored-by: Alex Boten <aboten@lightstep.com>
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.

4 participants