code review 6131044: misc/dashboard/codereview: more abbreviated modificatio...

misc/dashboard/codereview: more abbreviated modification duration.

[dsymonds, 2012-04-27 06:44:04]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/

[r, 2012-04-27 07:08:12]

LGTM
nice

http://codereview.appspot.com/6131044/diff/4001/misc/dashboard/codereview/das...
File misc/dashboard/codereview/dashboard/cl.go (right):

http://codereview.appspot.com/6131044/diff/4001/misc/dashboard/codereview/das...
misc/dashboard/codereview/dashboard/cl.go:50: // ShortOwner returns the CL's
owner, either as their email address
s/ShortOwner/DisplayOwnerName/ or something like that

http://codereview.appspot.com/6131044/diff/4001/misc/dashboard/codereview/das...
misc/dashboard/codereview/dashboard/cl.go:84: "h": time.Hour,
"d": 24*time.Hour

[dsymonds, 2012-04-27 07:10:15]

http://codereview.appspot.com/6131044/diff/4001/misc/dashboard/codereview/das...
File misc/dashboard/codereview/dashboard/cl.go (right):

http://codereview.appspot.com/6131044/diff/4001/misc/dashboard/codereview/das...
misc/dashboard/codereview/dashboard/cl.go:50: // ShortOwner returns the CL's
owner, either as their email address
On 2012/04/27 07:08:12, r wrote:
> s/ShortOwner/DisplayOwnerName/ or something like that

Went with DisplayOwner.

http://codereview.appspot.com/6131044/diff/4001/misc/dashboard/codereview/das...
misc/dashboard/codereview/dashboard/cl.go:84: "h": time.Hour,
On 2012/04/27 07:08:12, r wrote:
> "d": 24*time.Hour

Done.

[dsymonds, 2012-04-27 07:12:22]

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

misc/dashboard/codereview: more abbreviated modification duration.

R=golang-dev, r
CC=golang-dev
http://codereview.appspot.com/6131044

[rog, 2012-05-01 09:31:02]

http://codereview.appspot.com/6131044/diff/8002/misc/dashboard/codereview/das...
File misc/dashboard/codereview/dashboard/cl.go (right):

http://codereview.appspot.com/6131044/diff/8002/misc/dashboard/codereview/das...
misc/dashboard/codereview/dashboard/cl.go:89: d := time.Now().Sub(cl.Modified)
d := time.Since(cl.Modified)

http://codereview.appspot.com/6131044/diff/8002/misc/dashboard/codereview/das...
misc/dashboard/codereview/dashboard/cl.go:90: for suffix, u := range units {
this looks suspicious to me - won't it only work corrently if the units are
iterated in the order they are above (greatest-first)?
but maps are iterated in random order.

[dsymonds, 2012-05-01 11:31:04]

You'll find a later CL removed that map.