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

update to latest linter and tools #25

Merged
merged 3 commits into from
Aug 10, 2022
Merged

update to latest linter and tools #25

merged 3 commits into from
Aug 10, 2022

Conversation

pkwarren
Copy link
Contributor

Update connect-grpchealth-go to the latest golangci-lint, remove some
unnecessary //nolint comments, and fix the makefile to work with Go 1.19
(relative paths to executables are no longer allowed
https://go.dev/doc/go1.19#os-exec-path).

Update connect-grpchealth-go to the latest golangci-lint, remove some
unnecessary //nolint comments, and fix the makefile to work with Go 1.19
(relative paths to executables are no longer allowed
https://go.dev/doc/go1.19#os-exec-path).
- interfacer # deprecated by author
- ireturn # "accept interfaces, return structs" isn't ironclad
- lll # don't want hard limits for line length
- maintidx # covered by gocyclo
- maligned # readability trumps efficient struct packing
- nlreturn # generous whitespace violates house style
- nosnakecase # deprecated in https://github.com/golangci/golangci-lint/pull/3065
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will be deprecated in next golangci-lint but disabled for now because it causes lint errors on imported generated code: sivchari/nosnakecase#16

@@ -48,10 +50,3 @@ issues:
# Don't ban use of fmt.Errorf to create new errors, but the remaining
# checks from err113 are useful.
- "err113: do not define dynamic errors.*"

exclude-rules:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is working now without this exclusion.

@@ -6,7 +6,8 @@ SHELL := bash
MAKEFLAGS += --warn-undefined-variables
MAKEFLAGS += --no-builtin-rules
MAKEFLAGS += --no-print-directory
BIN=.tmp/bin
Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://go.dev/doc/go1.19#os-exec-path - We have to change the PATH to allow execution in go 1.19+.

@@ -37,15 +37,16 @@ func TestHealth(t *testing.T) {
server := httptest.NewUnstartedServer(mux)
server.EnableHTTP2 = true
server.StartTLS()
defer server.Close()
t.Cleanup(server.Close)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These tests work with t.Parallel() if we close the server in the cleanup method.

@pkwarren pkwarren merged commit 82e045b into main Aug 10, 2022
@pkwarren pkwarren deleted the pkw/go-1.19-lint-fixes branch August 10, 2022 15:02
akshayjshah pushed a commit that referenced this pull request Jul 26, 2023
Update connect-grpchealth-go to the latest golangci-lint, remove some
unnecessary //nolint comments, and fix the makefile to work with Go 1.19
(relative paths to executables are no longer allowed
https://go.dev/doc/go1.19#os-exec-path).
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.

None yet

2 participants