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

Ignore global vars prefixed with {E,e}rr #2

Closed
leonklingele opened this issue Sep 6, 2018 · 5 comments
Closed

Ignore global vars prefixed with {E,e}rr #2

leonklingele opened this issue Sep 6, 2018 · 5 comments

Comments

@leonklingele
Copy link
Contributor 

x.go:11::warning: ErrHandlerAlreadyExists is a global variable (gochecknoglobals)

It's a common practice to create global error variables such as the one above. It might be wise to not warn about them. What do you think?
@leighmcculloch
Copy link
Owner

Yeah that's a good point. We could ignore global variables starting with Err and that satisfy the error Interface.

@leighmcculloch
Copy link
Owner

Would you be up for taking a stab at this and opening a PR?

@leonklingele
Copy link
Contributor Author

leonklingele commented Sep 6, 2018 via email

leonklingele added a commit to leonklingele/gochecknoglobals that referenced this issue Sep 6, 2018
@leonklingele
Ignore global error variables (implements leighmcculloch#2)
1ca2dcc 
Error variables are variables starting with 'err' or 'Err' which
were made by either errors.New or fmt.Errorf.

There still is leeway for improvement such as checking if the
node implements the error interface. This would allow us to
whitelist custom error variables.
@leonklingele
Copy link
Contributor Author

For the records: This has been implemented in #3. Don't know why GH doesn't crosslink to it.

leonklingele added a commit to leonklingele/gochecknoglobals that referenced this issue Sep 6, 2018
@leonklingele
Ignore global error variables (implements leighmcculloch#2)
ff9c8da 
Error variables are variables starting with 'err' or 'Err' which
were made by either errors.New or fmt.Errorf.

There still is leeway for improvement such as checking if the
node implements the error interface. This would allow us to
whitelist custom error variables.
@leonklingele leonklingele mentioned this issue Sep 7, 2018
Merged
@leighmcculloch
Copy link
Owner

You need to reference this issue in the PR description for GH to crosslink it. Unfortunately the reference in the PR title doesn't crosslink. I just added the Fixes #2 to the PR description and it has linked it now.

leonklingele added a commit to leonklingele/gochecknoglobals that referenced this issue Sep 7, 2018
@leonklingele
Ignore global error variables
8710620 
Error variables are variables starting with 'err' or 'Err'.

This implements leighmcculloch#2.
leonklingele added a commit to leonklingele/gochecknoglobals that referenced this issue Sep 8, 2018
@leonklingele
Ignore global error variables
4886830 
Error variables are variables starting with 'err' or 'Err'.

This implements leighmcculloch#2.
leighmcculloch pushed a commit that referenced this issue Sep 8, 2018
@leonklingele @leighmcculloch
Ignore global error variables (implements #2) (#3)
c6d36d2 
Ignore global variables that look like errors because they start with 'err' or 'Err'.

Errors are a common thing to define at the package level, and are really used more like constants. Go doesn't allow an error to be defined as a constant though, so they are technically global variables. Since they aren't used to store state and having them defined at the package level is a common pattern, it doesn't make sense to prevent their existence.

There still is leeway for improvement such as checking if the node implements the error interface.

Fixes #2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@leighmcculloch @leonklingele