On Saturday, December 17, 2011, <dsymonds@golang.org> wrote: > LGTM > > > http://codereview.appspot.com/5491062/diff/4001/src/pkg/net/dial.go > File ...
13 years, 7 months ago
(2011-12-18 01:08:55 UTC)
#5
On Saturday, December 17, 2011, <dsymonds@golang.org> wrote:
> LGTM
>
>
> http://codereview.appspot.com/5491062/diff/4001/src/pkg/net/dial.go
> File src/pkg/net/dial.go (right):
>
>
http://codereview.appspot.com/5491062/diff/4001/src/pkg/net/dial.go#newcode83
> src/pkg/net/dial.go:83: t := time.NewTimer(timeout)
> On 2011/12/17 05:08:53, adg1 wrote:
>>
>> why not t.After? Doesn't using a ticker have the same drawbacks?
>
> The common case in this code is that the timeout isn't reached; using
> time.After would result in a goroutine and channel that hang around
> until the timeout is reached. If you were dialing a lot, that would get
> inefficient.
Oh, I misread this as NewTicker, not NewTimer. Definitely the right choice,
then.
From the name alone, I'd interpret DialWithin like it was dialing from a given local ...
13 years, 7 months ago
(2011-12-19 21:56:27 UTC)
#9
From the name alone, I'd interpret DialWithin like it was dialing from a
given local address ("dial within this subnet").
I'm happy with DialTimeout or DialWithTimeout. I also considered
DialBefore, but that sounds like it'd take an absolute time, not a duration.
Russ picked the name, IIRC. I'm not picky.
On Mon, Dec 19, 2011 at 1:51 PM, <r@golang.org> wrote:
>
>
http://codereview.appspot.com/**5491062/diff/4001/src/pkg/net/**dial.go<http:...
> File src/pkg/net/dial.go (right):
>
> http://codereview.appspot.com/**5491062/diff/4001/src/pkg/net/**
>
dial.go#newcode78<http://codereview.appspot.com/5491062/diff/4001/src/pkg/net/dial.go#newcode78>
> src/pkg/net/dial.go:78: func DialTimeout(net, addr string, timeout
> time.Duration) (Conn, error) {
> not enamored with the name. DialTimeout sounds like something that
> causes dial to timeout, not something that manages it.
> DialWithTimeout is better but longer.
> cleverer: DialWithin.
> reads like this
> DialWithin(net, addr, 10*time.Second)
>
> but if you prefer to keep DialTimeout, i won't press you.
>
>
http://codereview.appspot.com/**5491062/<http://codereview.appspot.com/5491062/>
>
Issue 5491062: code review 5491062: net: DialTimeout
(Closed)
Created 13 years, 7 months ago by bradfitz
Modified 13 years, 7 months ago
Reviewers:
Base URL:
Comments: 9