Skip to content
This repository was archived by the owner on Sep 9, 2020. It is now read-only.

Conversation

@jmank88
Copy link
Collaborator

@jmank88 jmank88 commented Oct 11, 2017

What does this do / why do we need it?

This PR refactors newVcsRemoteErrorOr and newVcsLocalErrorOr with the primary goal of fixing the bug of wrapping the error passed in and a secondary goal of drying up the usages.

What should your reviewer look out for in this PR?

Should we still wrap up the context errors, rather than return them bare?
There is a case context.Canceled, context.DeadlineExceeded: check for log suppression, otherwise I'm not sure the err needs to be bare.

@sdboyer
Copy link
Member

sdboyer commented Oct 11, 2017

Should we still wrap up the context errors, rather than return them bare?
There is a case context.Canceled, context.DeadlineExceeded: check for log suppression, otherwise I'm not sure the err needs to be bare.

without having looked at the actual code here, now that we've started using pkg/errors we need to be more diligent about calling errors.Cause() anywhere that we're doing error type/equality assertions.

@jmank88 jmank88 closed this Oct 12, 2017
@jmank88 jmank88 reopened this Oct 12, 2017
@sdboyer sdboyer merged commit 7b36282 into golang:master Oct 13, 2017
@jmank88 jmank88 deleted the vcs_error branch February 2, 2018 21:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants