-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Use error wrapping in Go 1.13 #2270
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
Comments
@ianlewis I want to try this issue. But I don't know where to start because this issue is related to all sorts of codes in this project. And I'm not familiar to the details of gVisor but I know what this issue expects. Could you guide me in a bit? |
@Yuya9786 It shouldn't be too involved. This is really just a placeholder issue to use return fmt.Errorf("some error: %v", err) The code would look like this return fmt.Errorf("some error: %w", err) You can search for some places where |
I got it. I'll fix all |
@Yuya9786 Excuse me, are you still working on this issue? |
@ryo-yamaoka I've already done to fix and succeeded to build, but I couldn't succeed to test because of timeout. |
@Yuya9786 I tried high spec machine, then succeeded to build and SPEC of build environment
make tests (so big log!)
|
By the way, when I run
|
@ryo-yamaoka Thank you for pointing out my mistakes, I fixed these.
There are many errors because of same reason. I posted same content in my pull request. (#7285 (comment)) |
I fixed it and then succeeded |
Okay thanks, I'll check tonight |
@Yuya9786 Me too test failed, but not timeout error. container-testsA lot of following errors.
syscall-testsA lot of following errors. very long
|
For now, your PR's CI failed by simple issue. |
@ryo-yamaoka Thank you for your help. |
Issue #2270 I changed `fmt.Errorf`s' `%v` to `%w` and succeeded to build, but I couldn't succeed to test because of timeout. My laptop specs might be not good enough. In Issue #6797, one person said 8-core-setup could pass most of tests (#6797 (comment)), but I can't afford of it. So could someone test this? FUTURE_COPYBARA_INTEGRATE_REVIEW=#7285 from Yuya9786:use-error-wrapping 03a4c5a PiperOrigin-RevId: 435254497
Is this issue still active? I'd like to try taking on this as my first issue |
I think my PR was approved by two reviewers and I resolved the conflicts once, but it's not been merged. |
hey @Yuya9786 thanks for the pr, it seems that there are still some conflicts that block the merge, e.g. VFS1 was completed removed at pkg/sentry/fs which your change are also applied to. Could you rebase your change and resolve those conflicts? Then we shall be able to merge the pull request. |
A friendly reminder that this issue had no activity for 120 days. |
Make use of the new error wrapping functionality and API to be able to check error types easier.
https://blog.golang.org/go1.13-errors
The text was updated successfully, but these errors were encountered: