code review 5696078: time: add a comment about how to use the Duration constants

time: add a comment about how to use the Duration constants

[r, 2012-02-26 08:59:19]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go/

[bradfitz, 2012-02-26 09:10:04]

Feels weird to write docs assuming the author is already in package "time".

I'd qualify all these symbols with "time.".  We do that for ExampleFoo
generally, don't we?


On Sun, Feb 26, 2012 at 12:59 AM, <r@golang.org> wrote:

> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> time: add a comment about how to use the Duration constants
>
> Please review this at
http://codereview.appspot.com/**5696078/<http://codereview.appspot.com/5696078/>
>
> Affected files:
>  M src/pkg/time/time.go
>
>
> Index: src/pkg/time/time.go
> ==============================**==============================**=======
> --- a/src/pkg/time/time.go
> +++ b/src/pkg/time/time.go
> @@ -384,6 +384,15 @@
>
>  // Common durations.  There is no definition for units of Day or larger
>  // to avoid confusion across daylight savings time zone transitions.
> +//
> +// To count the number of units in a duration, divide:
> +//     second := Duration(Second)
> +//     fmt.Print(int64(second/**Millisecond)) // prints 1000
> +//
> +// To convert an integer number of units to a duration, multiply:
> +//     seconds := 10
> +//     fmt.Print(Duration(seconds)***Second) // prints 10s
> +//
>  const (
>        Nanosecond  Duration = 1
>        Microsecond          = 1000 * Nanosecond
>
>
>

[r, 2012-02-26 09:23:31]

Hello golang-dev@googlegroups.com, bradfitz@golang.org (cc:
golang-dev@googlegroups.com),

Please take another look.

[r2, 2012-02-26 09:23:50]

On 26/02/2012, at 8:10 PM, Brad Fitzpatrick wrote:

> Feels weird to write docs assuming the author is already in package "time".
> 
> I'd qualify all these symbols with "time.".  We do that for ExampleFoo
generally, don't we?

I went back and forth on that issue. Most docs are in the package, but perhaps
this one is special.

-rob

[dsymonds, 2012-02-26 10:34:35]

You'd never write time.Duration(time.Second); time.Second is already that
type.

[r2, 2012-02-26 11:19:06]

sigh.

[r, 2012-02-26 11:19:38]

Hello golang-dev@googlegroups.com, bradfitz@golang.org, r@google.com,
dsymonds@golang.org (cc: golang-dev@googlegroups.com),

Please take another look.

[bradfitz, 2012-02-26 11:21:09]

http://codereview.appspot.com/5696078/diff/6/src/pkg/time/time.go
File src/pkg/time/time.go (right):

http://codereview.appspot.com/5696078/diff/6/src/pkg/time/time.go#newcode389
src/pkg/time/time.go:389: //	second := time.Second
"time." here

http://codereview.appspot.com/5696078/diff/6/src/pkg/time/time.go#newcode390
src/pkg/time/time.go:390: //	fmt.Print(int64(second/time.Millisecond)) // prints
1000
... and here

http://codereview.appspot.com/5696078/diff/6/src/pkg/time/time.go#newcode394
src/pkg/time/time.go:394: //	fmt.Print(Duration(seconds)*Second) // prints 10s
but not here.

[r, 2012-02-26 11:21:57]

Hello golang-dev@googlegroups.com, bradfitz@golang.org, r@google.com,
dsymonds@golang.org (cc: golang-dev@googlegroups.com),

Please take another look.

[bradfitz, 2012-02-26 11:23:54]

LGTM

[r2, 2012-02-26 11:24:10]

i wrote them, compiled them, ran them, took out the times, mailed them.
you said put the times back. then things started to go wrong. no excuses.

i've run them again. they work now. promise.

i suppose this is an argument for an executable example, and there should
perhaps be one, but there's also something to be said for simple code snippets
in the doc comment.

what a fiasco.

-rob

[r, 2012-02-26 11:24:56]

*** Submitted as http://code.google.com/p/go/source/detail?r=84582b0431a1 ***

time: add a comment about how to use the Duration constants

R=golang-dev, bradfitz, r, dsymonds
CC=golang-dev
http://codereview.appspot.com/5696078

[dsymonds, 2012-02-26 12:00:47]

LGTM

[crawshaw1, 2012-02-26 14:21:27]

http://codereview.appspot.com/5696078/diff/7002/src/pkg/time/time.go
File src/pkg/time/time.go (right):

http://codereview.appspot.com/5696078/diff/7002/src/pkg/time/time.go#newcode394
src/pkg/time/time.go:394: //	fmt.Print(time.Duration(seconds)*time.Second) //
prints 10s
equivalent: s/time.Duration(seconds)/seconds/

[bradfitzgoog, 2012-02-26 14:22:52]

http://codereview.appspot.com/5696078/diff/7002/src/pkg/time/time.go
File src/pkg/time/time.go (right):

http://codereview.appspot.com/5696078/diff/7002/src/pkg/time/time.go#newcode394
src/pkg/time/time.go:394: //	fmt.Print(time.Duration(seconds)*time.Second) //
prints 10s
On 2012/02/26 14:21:27, crawshaw1 wrote:
> equivalent: s/time.Duration(seconds)/seconds/

That's not the idiom, though.

[bradfitzgoog, 2012-02-26 14:25:57]

http://codereview.appspot.com/5696078/diff/7002/src/pkg/time/time.go
File src/pkg/time/time.go (right):

http://codereview.appspot.com/5696078/diff/7002/src/pkg/time/time.go#newcode394
src/pkg/time/time.go:394: //	fmt.Print(time.Duration(seconds)*time.Second) //
prints 10s
On 2012/02/26 14:22:53, bradfitzgoog wrote:
> On 2012/02/26 14:21:27, crawshaw1 wrote:
> > equivalent: s/time.Duration(seconds)/seconds/
> 
> That's not the idiom, though.

Not to mention your "equivalent" way wouldn't even compile (an int * an
time.Duration). But even if "secs" were a constant (not an int, as above), it
still wouldn't be the idiom.  We always write:

time.Duration(identifier) * time.Unit

And we tend to even include 1 when doing something like:

1 * time.Minute

[crawshaw1, 2012-02-26 14:54:56]

On Sun, Feb 26, 2012 at 9:25 AM,  <bradfitz@google.com> wrote:
>
> http://codereview.appspot.com/5696078/diff/7002/src/pkg/time/time.go
> File src/pkg/time/time.go (right):
>
>
http://codereview.appspot.com/5696078/diff/7002/src/pkg/time/time.go#newcode394
> src/pkg/time/time.go:394:
> //      fmt.Print(time.Duration(seconds)*time.Second) // prints 10s
> On 2012/02/26 14:22:53, bradfitzgoog wrote:
>>
>> On 2012/02/26 14:21:27, crawshaw1 wrote:
>> > equivalent: s/time.Duration(seconds)/seconds/
>
>
>> That's not the idiom, though.
>
>
> Not to mention your "equivalent" way wouldn't even compile (an int * an
> time.Duration). But even if "secs" were a constant (not an int, as
> above), it still wouldn't be the idiom.  We always write:
>
> time.Duration(identifier) * time.Unit
>
> And we tend to even include 1 when doing something like:
>
> 1 * time.Minute
>
> http://codereview.appspot.com/5696078/

Ah you're right, I took the easy but very different shortcut when
testing, 10*time.Second. I'll be quiet now.