[go] net/http: make Request.WithContext documentation less prescriptive
Damien Neil (Gerrit)
Damien Neil has uploaded this change for review.
net/http: make Request.WithContext documentation less prescriptive
WithContext makes a shallow copy of a Request, and Clone makes a
deep copy. Both set the context of the new request. The distinction
between the two is clear, and it doesn't seem useful or necessary
to say that "it's rare to need WithContext".
Also update a couple locations that mention WithContext to mention
Clone as well.
Fixes #53413.
Change-Id: I89e6ddebd7d5ca6573e522fe48cd7f50cc645cdd
---
M src/net/http/request.go
1 file changed, 21 insertions(+), 7 deletions(-)
diff --git a/src/net/http/request.go b/src/net/http/request.go
index d091f3c..2180cac 100644
--- a/src/net/http/request.go
+++ b/src/net/http/request.go
@@ -317,14 +317,14 @@
Response *Response
// ctx is either the client or server context. It should only
- // be modified via copying the whole Request using WithContext.
+ // be modified via copying the whole Request using Clone or WithContext.
// It is unexported to prevent people from using Context wrong
// and mutating the contexts held by callers of the same request.
ctx context.Context
}
// Context returns the request's context. To change the context, use
-// WithContext.
+// Clone or WithContext.
//
// The returned context is always non-nil; it defaults to the
// background context.
@@ -347,11 +347,6 @@
// For outgoing client request, the context controls the entire
// lifetime of a request and its response: obtaining a connection,
// sending the request, and reading the response headers and body.
-//
-// To create a new request with a context, use NewRequestWithContext.
-// To change the context of a request, such as an incoming request you
-// want to modify before sending back out, use Request.Clone. Between
-// those two uses, it's rare to need WithContext.
func (r *Request) WithContext(ctx context.Context) *Request {
if ctx == nil {
panic("nil context")
To view, visit change 412778. To unsubscribe, or for help writing mail filters, visit settings.
Damien Neil (Gerrit)
Attention is currently required from: Brad Fitzpatrick.
Patch set 1:Run-TryBot +1
David Ashby (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Damien Neil.
1 comment:
Patchset:
Anything else this needs to get merged now that we're outside the 1.19 freeze window, or are we just waiting for a review?
To view, visit change 412778. To unsubscribe, or for help writing mail filters, visit settings.
Brad Fitzpatrick (Gerrit)
Attention is currently required from: Damien Neil.
1 comment:
File src/net/http/request.go:
Patch Set #1, Line 351:
// To create a new request with a context, use NewRequestWithContext.I'd like to preserve this mention of NewRequestWithContext which is more efficient than using WithContext.
Maybe just drop the last sentence?
To view, visit change 412778. To unsubscribe, or for help writing mail filters, visit settings.
Brad Fitzpatrick (Gerrit)
Attention is currently required from: Damien Neil.
Patch set 1:Code-Review +2
Damien Neil (Gerrit)
Attention is currently required from: Damien Neil.
Damien Neil uploaded patch set #2 to this change.
The following approvals got outdated and were removed: Run-TryBot+1 by Damien Neil, TryBot-Result+1 by Gopher Robot
net/http: make Request.WithContext documentation less prescriptive
WithContext makes a shallow copy of a Request, and Clone makes a
deep copy. Both set the context of the new request. The distinction
between the two is clear, and it doesn't seem useful or necessary
to say that "it's rare to need WithContext".
Also update a couple locations that mention WithContext to mention
Clone as well.
Fixes #53413.
Change-Id: I89e6ddebd7d5ca6573e522fe48cd7f50cc645cdd
---
M src/net/http/request.go
1 file changed, 22 insertions(+), 5 deletions(-)
To view, visit change 412778. To unsubscribe, or for help writing mail filters, visit settings.
Damien Neil (Gerrit)
Patch set 1:Run-TryBot +1
1 comment:
File src/net/http/request.go:
Patch Set #1, Line 351:
// To create a new request with a context, use NewRequestWithContext.
I'd like to preserve this mention of NewRequestWithContext which is more efficient than using WithCo […]
Changed to preserve the reference to NewRequestWithContext, and to reference Request.Clone as an alternative that makes a deep copy.
To view, visit change 412778. To unsubscribe, or for help writing mail filters, visit settings.
Damien Neil (Gerrit)
Patch set 2:Run-TryBot +1
Robert Findley (Gerrit)
Attention is currently required from: Damien Neil.
Patch set 2:Code-Review +1
Damien Neil (Gerrit)
Damien Neil submitted this change.
1 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: src/net/http/request.go
Insertions: 3, Deletions: 0.
@@ -347,6 +347,9 @@
// For outgoing client request, the context controls the entire
// lifetime of a request and its response: obtaining a connection,
// sending the request, and reading the response headers and body.
+//
+// To create a new request with a context, use NewRequestWithContext.
+// To make a deep copy of a request with a new context, use Request.Clone.
func (r *Request) WithContext(ctx context.Context) *Request {
if ctx == nil {
panic("nil context")
```
net/http: make Request.WithContext documentation less prescriptive
WithContext makes a shallow copy of a Request, and Clone makes a
deep copy. Both set the context of the new request. The distinction
between the two is clear, and it doesn't seem useful or necessary
to say that "it's rare to need WithContext".
Also update a couple locations that mention WithContext to mention
Clone as well.
Fixes #53413.
Change-Id: I89e6ddebd7d5ca6573e522fe48cd7f50cc645cdd
Reviewed-on: https://go-review.googlesource.com/c/go/+/412778
Reviewed-by: Brad Fitzpatrick <brad...@golang.org>
Reviewed-by: Robert Findley <rfin...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
Run-TryBot: Damien Neil <dn...@google.com>
---
M src/net/http/request.go
1 file changed, 27 insertions(+), 5 deletions(-)
diff --git a/src/net/http/request.go b/src/net/http/request.go
index cead91d..5439cb3 100644
--- a/src/net/http/request.go
+++ b/src/net/http/request.go
@@ -317,14 +317,14 @@
Response *Response
// ctx is either the client or server context. It should only
- // be modified via copying the whole Request using WithContext.
+ // be modified via copying the whole Request using Clone or WithContext.
// It is unexported to prevent people from using Context wrong
// and mutating the contexts held by callers of the same request.
ctx context.Context
}
// Context returns the request's context. To change the context, use
-// WithContext.
+// Clone or WithContext.
//
// The returned context is always non-nil; it defaults to the
// background context.
@@ -349,9 +349,7 @@
// sending the request, and reading the response headers and body.
//
// To create a new request with a context, use NewRequestWithContext.
-// To change the context of a request, such as an incoming request you
-// want to modify before sending back out, use Request.Clone. Between
-// those two uses, it's rare to need WithContext.
+// To make a deep copy of a request with a new context, use Request.Clone.
func (r *Request) WithContext(ctx context.Context) *Request {
if ctx == nil {
panic("nil context")
To view, visit change 412778. To unsubscribe, or for help writing mail filters, visit settings.