Issue 7300099: code review 7300099: net: document OpError (Closed)
|
Created:
11 years, 2 months ago by bradfitz Modified:
11 years, 2 months ago Reviewers:
CC:
adg, rsc, golang-dev Visibility:
Public. |
Descriptionnet: document OpError
Fixes issue 4797
Patch Set 1 #Patch Set 2 : diff -r 6c6b3968b605 https://go.googlecode.com/hg/ #
Total comments: 12
Patch Set 3 : diff -r 8c44c45a208e https://go.googlecode.com/hg/ #MessagesTotal messages: 8
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://go.googlecode.com/hg/
Sign in to reply to this message.
Do we want to document on each function/method where this error type is returned? Currently we never mention it, anywhere. Mentioning it would be tantamount to guaranteeing it in the future, API-wise. On Tue, Feb 12, 2013 at 5:06 PM, <bradfitz@golang.org> wrote: > Reviewers: golang-dev_googlegroups.com, > > Message: > Hello golang-dev@googlegroups.com, > > I'd like you to review this change to > https://go.googlecode.com/hg/ > > > Description: > net: document OpError > > Fixes issue 4797 > > Please review this at https://codereview.appspot.**com/7300099/<https://codereview.appspot.com/7300... > > Affected files: > M src/pkg/net/net.go > > > Index: src/pkg/net/net.go > ==============================**==============================**======= > --- a/src/pkg/net/net.go > +++ b/src/pkg/net/net.go > @@ -276,11 +276,23 @@ > > var errMissingAddress = errors.New("missing address") > > +// An OpError is the error type returned by most functions in the net > +// package and describes an operation and address of an underlying > +// error. > type OpError struct { > - Op string > - Net string > + // Op is the operation which caused the error, such as "read", > + // "write", "dial", "shutdown", "listen", "sendfile", etc. > + Op string > + > + // Net is the network type on which this error occurred, > + // such as "tcp", "udp6", etc. > + Net string > + > + // Addr is the network address on which this error occurred. > Addr Addr > - Err error > + > + // Error is the underlying error. > + Err error > } > > func (e *OpError) Error() string { > > >
Sign in to reply to this message.
On 13 February 2013 12:09, Brad Fitzpatrick <bradfitz@golang.org> wrote: > Do we want to document on each function/method where this error type is > returned? > > Currently we never mention it, anywhere. > > Mentioning it would be tantamount to guaranteeing it in the future, > API-wise. > If it's worth mentioning it's worth guaranteeing. I think it's worth mentioning.
Sign in to reply to this message.
I don't believe we should bother to list all the places that return OpError. The main point of OpError is to implement net.Error where appropriate.
Sign in to reply to this message.
Okay, but what about this CL itself, documenting a public type? On Wed, Feb 13, 2013 at 2:36 PM, Russ Cox <rsc@golang.org> wrote: > I don't believe we should bother to list all the places that return > OpError. The main point of OpError is to implement net.Error where > appropriate. > > >
Sign in to reply to this message.
https://codereview.appspot.com/7300099/diff/2001/src/pkg/net/net.go File src/pkg/net/net.go (right): https://codereview.appspot.com/7300099/diff/2001/src/pkg/net/net.go#newcode279 src/pkg/net/net.go:279: // An OpError is the error type returned by most functions in the net s/An // https://codereview.appspot.com/7300099/diff/2001/src/pkg/net/net.go#newcode280 src/pkg/net/net.go:280: // package and describes an operation and address of an underlying s/ and/. It/ It describes the operation, network type, and address of an underlying error. https://codereview.appspot.com/7300099/diff/2001/src/pkg/net/net.go#newcode284 src/pkg/net/net.go:284: // "write", "dial", "shutdown", "listen", "sendfile", etc. Are there others? If not, remove the etc. If so, should we list all of them? https://codereview.appspot.com/7300099/diff/2001/src/pkg/net/net.go#newcode294 src/pkg/net/net.go:294: // Error is the underlying error. I'm not sure if it should be "underlying" or "specific"?
Sign in to reply to this message.
LGTM https://codereview.appspot.com/7300099/diff/2001/src/pkg/net/net.go File src/pkg/net/net.go (right): https://codereview.appspot.com/7300099/diff/2001/src/pkg/net/net.go#newcode284 src/pkg/net/net.go:284: // "write", "dial", "shutdown", "listen", "sendfile", etc. On 2013/02/14 06:39:55, adg wrote: > Are there others? If not, remove the etc. If so, should we list all of them? Let's not list them all. `such as "read" or "write".` seems fine to me. https://codereview.appspot.com/7300099/diff/2001/src/pkg/net/net.go#newcode288 src/pkg/net/net.go:288: // such as "tcp", "udp6", etc. such as "tcp" or "udp6". https://codereview.appspot.com/7300099/diff/2001/src/pkg/net/net.go#newcode294 src/pkg/net/net.go:294: // Error is the underlying error. On 2013/02/14 06:39:55, adg wrote: > I'm not sure if it should be "underlying" or "specific"? how about Err is the error that occurred during the operation. note s/Error/Err/
Sign in to reply to this message.
*** Submitted as https://code.google.com/p/go/source/detail?r=9ab99f5d4a82 *** net: document OpError Fixes issue 4797 R=adg, rsc CC=golang-dev https://codereview.appspot.com/7300099 https://codereview.appspot.com/7300099/diff/2001/src/pkg/net/net.go File src/pkg/net/net.go (right): https://codereview.appspot.com/7300099/diff/2001/src/pkg/net/net.go#newcode279 src/pkg/net/net.go:279: // An OpError is the error type returned by most functions in the net On 2013/02/14 06:39:55, adg wrote: > s/An // Done. https://codereview.appspot.com/7300099/diff/2001/src/pkg/net/net.go#newcode280 src/pkg/net/net.go:280: // package and describes an operation and address of an underlying On 2013/02/14 06:39:55, adg wrote: > s/ and/. It/ > > It describes the operation, network type, and address of an underlying error. Done. https://codereview.appspot.com/7300099/diff/2001/src/pkg/net/net.go#newcode284 src/pkg/net/net.go:284: // "write", "dial", "shutdown", "listen", "sendfile", etc. On 2013/02/14 15:18:43, rsc wrote: > On 2013/02/14 06:39:55, adg wrote: > > Are there others? If not, remove the etc. If so, should we list all of them? > > Let's not list them all. `such as "read" or "write".` seems fine to me. Done. https://codereview.appspot.com/7300099/diff/2001/src/pkg/net/net.go#newcode288 src/pkg/net/net.go:288: // such as "tcp", "udp6", etc. On 2013/02/14 15:18:43, rsc wrote: > such as "tcp" or "udp6". Done. https://codereview.appspot.com/7300099/diff/2001/src/pkg/net/net.go#newcode294 src/pkg/net/net.go:294: // Error is the underlying error. On 2013/02/14 15:18:43, rsc wrote: > On 2013/02/14 06:39:55, adg wrote: > > I'm not sure if it should be "underlying" or "specific"? > > how about > > Err is the error that occurred during the operation. > > note s/Error/Err/ Done.
Sign in to reply to this message.
|
