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.

Sign up for GitHub

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

Jump to bottom

Update gochecknoglobals, use source analyzer #1422

Merged
merged 9 commits into from Oct 15, 2020
Merged

Update gochecknoglobals, use source analyzer #1422

merged 9 commits into from Oct 15, 2020

Conversation

bombsimon
Copy link
Member

@bombsimon bombsimon commented Oct 10, 2020 •
edited

This will use the latest version of gochecknoglobals and the analyzer that's been created. Closes #1393

This bump includes two changes:

  1. The error message goes from 'foo' is a global variable to foo is a global variable.
  2. Global variables assigned from regexp.MustCompile() are now allowed

I'm a bit confused regarding 1 since the old code doesn't add backticks and the source which this is copied from didn't do so previously either.

@bombsimon bombsimon requested a review from a team October 10, 2020 11:19
vovapi
vovapi approved these changes Oct 11, 2020
View reviewed changes
Copy link
Contributor

@vovapi vovapi left a comment

Choose a reason for hiding this comment

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

LGTM!

@bombsimon
Copy link
Member Author

Regarding 1, I totally missed the call to formatCode(). Seems like golangci-lint does this for al ot of linters. Anyone knows why? Would it be a problem to stop putting the variable name in backticks? I guess this will affect the output when used in the GitHub action.

Also noticed that this will never lint test files so I will add a patch for this asap.

vovapi
vovapi requested changes Oct 12, 2020
View reviewed changes
pkg/golinters/gochecknoglobals.go Outdated Show resolved Hide resolved
@bombsimon
Copy link
Member Author

Thanks for pointing out the faulty way to enforce the flag! Will patch this, are you OK with the change around formatCode()?

@bombsimon
Copy link
Member Author

Ping @vovapi @sayboras

@vovapi
Copy link
Contributor

vovapi commented Oct 15, 2020

Are you OK with the change around formatCode()

I'm ok with it.

sayboras
sayboras approved these changes Oct 15, 2020
View reviewed changes
Copy link
Member

@sayboras sayboras left a comment

Choose a reason for hiding this comment

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

LGTM 💯

@sayboras sayboras merged commit a8b7b00 into golangci:master Oct 15, 2020
@thaJeztah thaJeztah mentioned this pull request Oct 16, 2020
Open
@ldez ldez added the linter: update version Update version of linter label Dec 7, 2020
@bombsimon bombsimon mentioned this pull request Feb 21, 2021
Closed
@ldez ldez added this to the v1.32 milestone Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vovapi vovapi vovapi approved these changes

@sayboras sayboras sayboras approved these changes

Assignees
No one assigned
Labels
linter: update version Update version of linter
Projects
None yet
Milestone
v1.32
Development

Successfully merging this pull request may close these issues.

Update gochecknoglobals
4 participants
@bombsimon @vovapi @sayboras @ldez