Proposal: Add time.Until() to the time package #14595

Closed
SamWhited opened this Issue Mar 2, 2016 · 13 comments

Projects

None yet

8 participants

@SamWhited
Contributor

I'd like to propose that a time.Until(t time.Time) time.Duration function be added to the time package to compliment the existing Since() shortcut. This would make writing expressions with an expiration time a bit more readable:

<-After(time.Until(expirationTime))

vs.

<-After(expirationTime.Sub(time.Now()))

While it's still fairly obvious what the second one does, it takes a little longer to recognize "sub" as subtraction than just seeing the symbol. Also keeping time expressions more or less readable as english is a nice benefit of having the until shortcut (as you can do with the existing since function).

If this accepted, I've got a CL here for review.

@minux minux added the Proposal label Mar 2, 2016
@gopherbot

CL https://golang.org/cl/20118 mentions this issue.

@bradfitz
Member
bradfitz commented Mar 2, 2016

Does it pay for itself?

I see only 7 potential users in the standard library:

$ git grep -E 'Sub\(time.Now'
src/crypto/tls/tls.go:          deadlineTimeout := dialer.Deadline.Sub(time.Now())
src/net/dial_gen.go:    timeout := deadline.Sub(time.Now())
src/net/dial_test.go:           case <-time.After(deadline.Sub(time.Now())):
src/net/fd_poll_runtime.go:     diff := int64(t.Sub(time.Now()))
src/net/http/client.go: timer := time.NewTimer(deadline.Sub(time.Now()))
src/net/http/serve_test.go:                     if remaining := c.rd.Sub(time.Now()); remaining < cue {
src/net/lookup.go:      timeout := deadline.Sub(time.Now())
@SamWhited
Contributor

That's 7 places in the standard library that would be somewhat more readable; however, it's a public API so I'm sure there are many more places where it would be useful outside the standard library too. I'd say it pays for itself in adding simplicity, but maybe I'm alone in that?

@bradfitz
Member
bradfitz commented Mar 2, 2016

We often use the standard library as a proxy for how common a pattern is. Yes, surely there are more than 7 globally. But the question is how often rate-wise this occurs.

@SamWhited
Contributor

I see; I'd suspect a lot (actually, 7 seems like a pretty large number for the handful of packages in the standard library that will use this sort of thing).

For the record, I see 22 uses of Since in the standard library, which is more, but still not a huge number.

@minux
Member
minux commented Mar 2, 2016
@bradfitz bradfitz modified the milestone: Unplanned Apr 7, 2016
@extemporalgenome

How is this different than -1 * time.Since(expirationTime) or -time.Since(expirationTime) ?

@SamWhited
Contributor

How is this different than -1 * time.Since(expirationTime) or -time.Since(expirationTime)?

It's not, this would be for readability (in the same way that time.Since improves readability over just doing time.Now().Sub(t))

@bradfitz
Member

@campoy, can you query the Github corpus in BigQuery and see if this proposal would be worthwhile?

@campoy
Member
campoy commented Jul 27, 2016

There's in total around 2000 repos (counting forks only once) that could benefit from this feature.

SELECT REGEXP_EXTRACT(repo_name, r'.*/(.*)') as project, FLOOR(COUNT(*) / COUNT(DISTINCT repo_name)) as n, FIRST(line) as sample
FROM (
  SELECT id, split(content, '\n') as line
  FROM [campoy-github:go_files.contents]
  HAVING line CONTAINS '.Sub(time.Now())'
) as contents JOIN [campoy-github:go_files.files] as files
ON contents.id = files.id
GROUP BY project
ORDER BY n DESC

The 10 projects that would benefit the most are

Row project                     n           sample   
1   robin                       62.0        time.Sleep(expectedExp.Sub(time.Now()) - 500*time.Millisecond)   
2   contrib                     26.0        deadlineTimeout := dialer.Deadline.Sub(time.Now())   
3   j2                          26.0        time.Sleep(expectedExp.Sub(time.Now()) - 500*time.Millisecond)   
4   gitarchive                  20.0        c.traceInfo.firstLine.deadline = deadline.Sub(time.Now())    
5   megacfs                     20.0        sleep := nextRun.Sub(time.Now())     
6   microcosm                   20.0        d := deadline.Sub(time.Now())    
7   concourse-pipeline-resource 20.0        req.TimeoutSeconds = proto.Float64(cn.readDeadline.Sub(time.Now()).Seconds())    
8   etcd2-bootstrapper          20.0        Timeout: d.Sub(time.Now()),  
9   zypper-docker               18.0        timeout = deadline.Sub(time.Now())   
10  doit                        16.0        Timeout: d.Sub(time.Now()), 

In my opinion there's some projects that could benefit of this, but it's clearly not a high priority addition to the stdlib.

@adg
Contributor
adg commented Aug 29, 2016

I'm in favor of this. time.Since is a great API. time.Until is a fitting parallel.

@gopherbot gopherbot pushed a commit that closed this issue Aug 30, 2016
@SamWhited @bradfitz SamWhited + bradfitz time: Add Until helper function
Adds an Until() function that returns the duration until the given time.
This compliments the existing Since() function and makes writing
expressions that have expiration times more readable; for example:

    <-After(time.Until(connExpires)):

Fixes #14595

Change-Id: I87998a924b11d4dad5512e010b29d2da6b123456
Reviewed-on: https://go-review.googlesource.com/20118
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
67ea710
@gopherbot gopherbot closed this in 67ea710 Aug 30, 2016
@gopherbot gopherbot pushed a commit that referenced this issue Aug 30, 2016
@bradfitz bradfitz all: use time.Until where applicable
Updates #14595

Change-Id: Idf60b3004c7a0ebb59dd48389ab62c854069e09f
Reviewed-on: https://go-review.googlesource.com/28073
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
298791a
@s7v7nislands

should change milestone to go1.8? or how to set the milestone label?

@adg
Contributor
adg commented Aug 31, 2016

@s7v7nislands it doesn't matter now. The issue is resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment