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

More complicated linting #1524

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

More complicated linting #1524

wants to merge 1 commit into from

Conversation

ptman
Copy link
Contributor

@ptman ptman commented Jan 29, 2021

I expect there will be questions and I'll have to reverse some of the changes.

@notzippy
Copy link
Collaborator

This seems to have errors with the results class

./results.go:146:25: Errorf format %w has arg err of wrong type interface{}
./results.go:194:10: Errorf format %w has arg rerr of wrong type interface{}
./results.go:211:10: Errorf format %w has arg rerr of wrong type interface{}

@ptman
Copy link
Contributor Author

ptman commented Feb 26, 2021

Thanks for the review. I was misled by err := recover(), but recover returns interface{}, not error.

@notzippy
Copy link
Collaborator

notzippy commented Mar 6, 2022

@brendensoares Thoughts on this?

@brendensoares
Copy link
Member

This looks good, but we'll defer to next release pending testing/verification.

@brendensoares brendensoares added the status-needs-testing Code complete, needs peer verification label Apr 12, 2022
Checked linter warnings using:

     CGO_ENABLED=0 golangci-lint run --enable-all --disable testpackage,paralleltest,funlen,dupl,exhaustive,nakedret,noctx,nlreturn,gomnd,gochecknoglobals,gochecknoinits,interfacer,exhaustivestruct,wsl,lll,nestif,godox,gocyclo,gocognit,gomodguard,maligned,stylecheck,gosec,forbidigo,goconst,wrapcheck,staticcheck ./...

- Got rid of dynamic errors (errors.New())
- Using errors.Is() / errors.As() to test errors
- Added t.Helper() to test helpers
- Fixed InMemoryCache (use pointer receiver, don't copy or locks break)
- Simplified code in various places
- Aligned closer to standards
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status-needs-testing Code complete, needs peer verification
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants