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

Use Go 1.13 error wrapping #2308

Closed
joelanford opened this issue Dec 10, 2019 · 4 comments · Fixed by #2355
Closed

Use Go 1.13 error wrapping #2308

joelanford opened this issue Dec 10, 2019 · 4 comments · Fixed by #2355
Assignees
Labels
area/dependency Issues or PRs related to dependency changes good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines.

Comments

@joelanford
Copy link
Member

Now that we've upgraded to Go 1.13, we should find instances in our code base of:

  1. errors.Wrap(), errors.Wrapf(), and replace them with fmt.Errorf() with %w.
  2. fmt.Errorf() calls that wrap an error, but use %s or %v, and replace them with fmt.Errorf() with %w
  3. Any other uses of the github.com/pkg/errors package should be transitioned over to Go's standard library errors package, if possible.

All fmt.Errorf() calls that wrap an error should look like the following:

fmt.Errorf("some context about my error: %w", err)
  • %w should be preceded by : (colon space)
  • %w should not be in parentheses
@joelanford joelanford added good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. area/dependency Issues or PRs related to dependency changes labels Dec 10, 2019
@nrchakradhar
Copy link
Contributor

@joelanford I can take a shot at this. This will be my first contribution and hence I may take some time. Please let me know if its fine.

@joelanford
Copy link
Member Author

@nrchakradhar Awesome thanks! We would welcome this contribution!

@nrchakradhar
Copy link
Contributor

@joelanford I hope I have understood the changes to be done. I have written a script that does the change for fmt.ErrorF. I will put some samples here tomorrow. If it looks fine, I will commit the changes. Thanks for providing me an opportunity to contribute.

@nrchakradhar
Copy link
Contributor

nrchakradhar commented Dec 22, 2019

@joelanford
Following are some of the sample changes. If its fine, I will submit the changes for all files.
About 69 files with about 180 changes will be modified.

err other Changes done
%v none operator-sdk\cmd\operator-sdk\printdeps\cmd.go
LINE:  47
return fmt.Errorf("print deps failed: %v", err)
return fmt.ErrorF("print deps failed: %w", err)'
%v %v, (%v), %s, %q operator-sdk\test\test-framework\test\e2e\memcached_test.go
LINE:  91
return fmt.Errorf("could not update memcached CR %q: %v", key, err)
return fmt.ErrorF("could not update memcached CR %q: %w", key, err)
(%v) none operator-sdk\cmd\operator-sdk\add\api.go
LINE:  130
return fmt.Errorf("api scaffold failed: (%v)", err)
return fmt.ErrorF("api scaffold failed: %w", err)
(%v) %v, %s, (%v) operator-sdk\cmd\operator-sdk\add\api.go
operator-sdk\cmd\operator-sdk\build\cmd.go
LINE:  137
return fmt.Errorf("failed to output build image %s: (%v)", image, err)
return fmt.ErrorF("failed to output build image %s: %w", image, err)
%s none operator-sdk\internal\scaffold\helm\chart.go
LINE:  119
return nil, nil, fmt.Errorf("failed to create helm-charts directory: %s", err)
return nil, nil, fmt.ErrorF("failed to create helm-charts directory: %w", err)

nrchakradhar added a commit to nrchakradhar/operator-sdk that referenced this issue Dec 28, 2019
Use fmt.Errorf as per 1.13. Changed errors.Wrap and errors.Wrapf to use fmt.Errorf.

Fixes operator-framework#2308
nrchakradhar added a commit to nrchakradhar/operator-sdk that referenced this issue Dec 29, 2019
Use fmt.Errorf as per 1.13. Changed errors.Wrap and errors.Wrapf to use fmt.Errorf.

Fixes operator-framework#2308
nrchakradhar added a commit to nrchakradhar/operator-sdk that referenced this issue Jan 8, 2020
Use fmt.Errorf as per 1.13. Changed errors.Wrap and errors.Wrapf to use fmt.Errorf.

Fixes operator-framework#2308
nrchakradhar added a commit to nrchakradhar/operator-sdk that referenced this issue Jan 12, 2020
Use fmt.Errorf as per 1.13. Changed errors.Wrap and errors.Wrapf to use fmt.Errorf.

Fixes operator-framework#2308
nrchakradhar added a commit to nrchakradhar/operator-sdk that referenced this issue Jan 14, 2020
Use fmt.Errorf as per 1.13. Changed errors.Wrap and errors.Wrapf to use fmt.Errorf.

Fixes operator-framework#2308
nrchakradhar added a commit to nrchakradhar/operator-sdk that referenced this issue Jan 16, 2020
Use fmt.Errorf as per 1.13. Changed errors.Wrap and errors.Wrapf to use fmt.Errorf.

Fixes operator-framework#2308
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dependency Issues or PRs related to dependency changes good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants