Skip to content
This repository has been archived by the owner on Dec 1, 2021. It is now read-only.

Wrap argument name cause could be err #32

Closed
matryer opened this issue May 24, 2016 · 3 comments
Closed

Wrap argument name cause could be err #32

matryer opened this issue May 24, 2016 · 3 comments

Comments

@matryer
Copy link
Contributor

matryer commented May 24, 2016

This might seem a little strange, but I think cause as an argument name to Wrap and Wrapf is unnecessary. I wonder if err might be good enough?

Here are some reasons:

  1. it's obvious that we're wrapping an error that caused the need to add context in the first place
  2. there's only one argument of type error
  3. it is possible the argument is not the 'root' cause, so in a sense, isn't the 'cause'
  4. IDE plugins will autofil and most of the time, the variable name would be err so you could just tab past it

Very minor issue, and mainly I'm trying to save myself two key-presses each time I use this (which I will every day).

@davecheney
Copy link
Member

I buy the argument in your third point. Feel free to send a PR, don't
forget to update the docs and the package doc and the README. sorry for the
duplication

On Wed, 25 May 2016, 09:12 Mat Ryer, notifications@github.com wrote:

This might seem a little strange, but I think cause as an argument name
to Wrap and Wrapf is unnecessary. I wonder if err might be good enough?

Here are some reasons:

  • it's obvious that we're wrapping an error that caused the need to
    add context
  • there's only one argument of type error
  • it is possible the argument is not the 'root' cause, so in a
    sense, isn't the 'cause'
  • IDE plugins will autofil and most of the time, the variable name
    would be err so you can just tab past it

Very minor issue, and mainly I'm trying to save myself two key-presses
each time I use this (which I will every day).


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#32

@davecheney
Copy link
Member

Actually, the error passed to Wrap and friends it's the cause. Error x is
caused by error y, and error y may either be the root cause or be
symptomatic of another.

But still send a PR to rename cause to err in the formal parameters. Its
not important enough to be different.

On Wed, 25 May 2016, 09:18 Dave Cheney, dave@cheney.net wrote:

I buy the argument in your third point. Feel free to send a PR, don't
forget to update the docs and the package doc and the README. sorry for the
duplication

On Wed, 25 May 2016, 09:12 Mat Ryer, notifications@github.com wrote:

This might seem a little strange, but I think cause as an argument name
to Wrap and Wrapf is unnecessary. I wonder if err might be good enough?

Here are some reasons:

  • it's obvious that we're wrapping an error that caused the need to
    add context
  • there's only one argument of type error
  • it is possible the argument is not the 'root' cause, so in a
    sense, isn't the 'cause'
  • IDE plugins will autofil and most of the time, the variable name
    would be err so you can just tab past it

Very minor issue, and mainly I'm trying to save myself two key-presses
each time I use this (which I will every day).


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#32

@matryer
Copy link
Contributor Author

matryer commented May 24, 2016 via email

matryer added a commit to matryer/errors that referenced this issue May 24, 2016
davecheney added a commit that referenced this issue May 26, 2016
`errors.wrap` existed to avoid the conflict between the type,
`errors.cause` and the formal parameter `cause`. The parameter was
renamed to err in #32 so there is no need for the helper.
YuJuncen pushed a commit to YuJuncen/errors that referenced this issue Apr 15, 2021
move tiup/components/errdoc/errdoc-gen to errors/errdoc-gen
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants