-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Replace all instances of fmt.Errorf(%v) with fmt.Errorf(%w) #21551
Replace all instances of fmt.Errorf(%v) with fmt.Errorf(%w) #21551
Conversation
IIRC, golangci-lint may have a linter to detect this. Not sure I find it now. |
Agreed. But please only once this PR is merged. |
Found using `find . -type f -name '*.go' -print -exec vim {} -c ':%s/fmt\.Errorf(\(.*\)%v\(.*\)err/fmt.Errorf(\1%w\2err/g' -c ':wq' \;`
bbd2286
to
653678e
Compare
Oh gods there are some delightfully unenlightening error messages there. @wxiaoguang I think you're right we're just going to have to go down the route of adding the stack trace to these. |
What exactly do you mean? |
Found the linter: https://github.com/polyfloyd/go-errorlint You can just add |
@delvh not really related to this PR but look at some of the errors reported, e.g. in notification.go:loadRepo(...): if n.Repository == nil {
n.Repository, err = repo_model.GetRepositoryByIDCtx(ctx, n.RepoID)
if err != nil {
return fmt.Errorf("getRepositoryByID [%d]: %w", n.RepoID, err) // <- BUT WHY WAS I TRYING TO GET THIS?
}
} If we think about this message a bit more - The error that GetRepositoryByIDCtx should return should already be We're just adding the wrong context and doing so at every level. I should add that at least this error is giving the context of the RepoID that is failing to be got from the DB. Many don't even provide that kind of information, e.g.: // DeleteGPGKey deletes GPG key information in database.
func DeleteGPGKey(doer *user_model.User, id int64) (err error) {
key, err := GetGPGKeyByID(id)
if err != nil {
if IsErrGPGKeyNotExist(err) {
return nil
}
return fmt.Errorf("GetPublicKeyByID: %w", err) // WHICH BLOODY KEY?
} |
I did test |
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.
as comment
|
Signed-off-by: Andrew Thornton <art27@cantab.net>
Generally L-G-T-M. CI failures are related, golangci-lint does very good job and catches mismatched formatting arguments. |
Co-authored-by: zeripath <art27@cantab.net>
Signed-off-by: Andrew Thornton <art27@cantab.net>
There was an incorrect whitespace change in blame_test.go. |
What happened to the CI? |
* upstream/main: [skip ci] Updated translations via Crowdin Add sqlite vscode extension to Gitpod configuration (go-gitea#21552) Replace all instances of fmt.Errorf(%v) with fmt.Errorf(%w) (go-gitea#21551) Fix package access for admins and inactive users (go-gitea#21580) Allow for resolution of NPM registry paths that match upstream (go-gitea#21568) Added missing headers on user packages page (go-gitea#21172) Record OAuth client type at registration (go-gitea#21316)
Found using
find . -type f -name '*.go' -print -exec vim {} -c ':%s/fmt\.Errorf(\(.*\)%v\(.*\)err/fmt.Errorf(\1%w\2err/g' -c ':wq' \;