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

fix: don't exclude fmt.Errorf #235

Merged
merged 1 commit into from
Nov 9, 2023
Merged

fix: don't exclude fmt.Errorf #235

merged 1 commit into from
Nov 9, 2023

Conversation

thehowl
Copy link
Contributor

@thehowl thehowl commented Oct 27, 2023

... and run gofumpt on excludes.go, and update dependencies. (errcheck is failing for me, with error package "reflect" without types was imported from "github.com/gnolang/gno/gnovm/pkg/gnolang" -- I think this is due to type parameters, and updating x/tools fixes the issue).

The reason why I think we should not exclude fmt.Errorf is that, by making sure it is used, we catch wrong code like:

if err != nil {
	fmt.Errorf("invalid file: %w", err)
}

This is obviously meant to return the error, but it's not.

If the current behaviour is to be considered correct, then errors.New should also be excepted.

kisielk
kisielk previously approved these changes Nov 2, 2023
@kisielk
Copy link
Owner

kisielk commented Nov 2, 2023

@thehowl can you please rebase on the latest master branch? I updated the test matrix to test with more recent versions of go and dropped < 1.18 support

@thehowl
Copy link
Contributor Author

thehowl commented Nov 9, 2023

@kisielk rebased ;)

@kisielk kisielk merged commit a75c8d0 into kisielk:master Nov 9, 2023
3 checks passed
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.

2 participants