-
Notifications
You must be signed in to change notification settings - Fork 7
fix: nakedret and exhaustive config #199
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
base: main
Are you sure you want to change the base?
Conversation
max-func-lines: 60 | ||
exhaustive: | ||
# Default: false | ||
default-signifies-exhaustive: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this one I'm not so sure about though...
on one hand, most of the times we do use default
, it really is meant to cover all other options
but some times, the proper way might be to cover all the options, and make default
error (as in, someone somehow provided an unsupported option)
not sure how common the later is, but I have seen the first in our codebase quite a few times
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this lint tbh, I feel skip default
when good mapped cases are settled is better. It's the pattern in many languages that prefer explicity over implicity (Python, Rust, ...). Since can lead to mistakes obscure case switchs
linters-settings: | ||
nakedret: | ||
# Default: 30 | ||
max-func-lines: 60 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this one makes sense to me
we could also forbid naked returns 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a good middle ground, personally.
@@ -1,3 +1,5 @@ | |||
# yaml-language-server: $schema=https://golangci-lint.run/jsonschema/golangci.v1.jsonschema.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's a v2 now, this prevents the editor from complaining about it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm ok with both changes
I propose we give it a bit more lines before it starts complaining.