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

Swap to fmt.Errorf in jobobject package #1353

Merged
merged 2 commits into from
Apr 14, 2022

Conversation

dcantah
Copy link
Contributor

@dcantah dcantah commented Apr 14, 2022

This change swaps to fmt.Errorf for wrapped errors in the jobobject package from errors.Wrap. We'd had talks of moving this code to winio where we've removed our pkg/errors dependency so this would make the port easier.

We should probably do this for most things to see if we can get rid of our pkg/errors use, the only thing I'm not sure on is we use the stack embedding errors.Wrap provides for the gcs https://github.com/microsoft/hcsshim/blob/master/internal/guest/gcserr/errors.go#L57-L88

This change swaps to fmt.Errorf for wrapped errors in the jobobject package
from errors.Wrap. We'd had talks of moving this code to winio where we've
removed our pkg/errors dependency so this would make the port easier.

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
@dcantah dcantah requested a review from a team as a code owner April 14, 2022 07:50
Copy link
Contributor

@helsaawy helsaawy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't comment on them, but can you replace the "github.com/pkg/errors" imports with the vanilla "errors" as well?

@dcantah
Copy link
Contributor Author

dcantah commented Apr 14, 2022

I can't comment on them, but can you replace the "github.com/pkg/errors" imports with the vanilla "errors" as well?

Oh good call

Swap imports to "errors" for errors.New usage also

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
@dcantah
Copy link
Contributor Author

dcantah commented Apr 14, 2022

@helsaawy Done

@dcantah dcantah merged commit ccec73f into microsoft:master Apr 14, 2022
dcantah added a commit to dcantah/hcsshim that referenced this pull request May 19, 2022
* Swap to fmt.Errorf in jobobject package

This change swaps to fmt.Errorf for wrapped errors in the jobobject package
from errors.Wrap. We'd had talks of moving this code to winio where we've
removed our pkg/errors dependency so this would make the port easier.

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
(cherry picked from commit ccec73f)
Signed-off-by: Daniel Canter <dcanter@microsoft.com>
anmaxvl added a commit that referenced this pull request Feb 7, 2023
Sync ADO with upstream to enable including test GCS binaries as
part of dev-pipeline

Related work items: #1311, #1322, #1341, #1343, #1345, #1347, #1348, #1350, #1353, #1354, #1355, #1358, #1361, #1365, #1368, #1369, #1370
princepereira pushed a commit to princepereira/hcsshim that referenced this pull request Aug 29, 2024
* Swap to fmt.Errorf in jobobject package

This change swaps to fmt.Errorf for wrapped errors in the jobobject package
from errors.Wrap. We'd had talks of moving this code to winio where we've
removed our pkg/errors dependency so this would make the port easier.

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
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.

3 participants