chore: Remove varnamelen linter #854
Conversation
|
Also the linter doesn't say where the lint failure is" https://github.com/coder/coder/runs/5818950685?check_suite_focus=true |
Codecov Report
@@ Coverage Diff @@
## main #854 +/- ##
==========================================
- Coverage 66.08% 65.87% -0.21%
==========================================
Files 202 202
Lines 13209 13209
Branches 87 87
==========================================
- Hits 8729 8702 -27
- Misses 3597 3619 +22
- Partials 883 888 +5
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
I've never felt like our variable names in v1 were too short in my opinion.
I've had this bite me a few times, I think it's too sensitive. I agree with it in theory but it wouldn't let me use a single character variable for a 4 line middleware function. Idiomatic Go would say that is totally fine as long as it's not a super long function and doesn't leave scope.
I generally don't love using # of lines as the metric for this.
|
I'd prefer for us organic life forms to make the decision on this sort of thing. |
There was a problem hiding this comment.
I've personally found this to be quite helpful in producing readable code. I think that edge-case of specific types matched with specific names can be helpful to declare.
It's an arbitrary position, but I'd rather have consistency with rw http.ResponseWriter over a mix of w and rw than the shorter name.
|
My comment doesn't need to block merge. Mostly everyone agrees, so if it becomes a problem we can add it back. |
Golang prefers short var names in local scope. I think this is not idiomatic and excessive. I'd only assert this when the var exceeds local scope, but that is not supported.
https://github.com/golang/go/wiki/CodeReviewComments#variable-names