Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(303)

Issue 179079: code review 179079: Added ISO 8601 time format output.

Can't Edit
Can't Publish+Mail
Start Review
Created:
14 years, 3 months ago by benolive
Modified:
14 years, 3 months ago
Reviewers:
rsc
CC:
r, rsc, golang-dev
Visibility:
Public.

Description

Added ISO 8601 time format output. Fixes issue 431.

Patch Set 1 #

Patch Set 2 : code review 179079: Added ISO 8601 time format output. #

Patch Set 3 : code review 179079: Added ISO 8601 time format output. #

Total comments: 3

Patch Set 4 : code review 179079: Added ISO 8601 time format output. #

Total comments: 5

Patch Set 5 : code review 179079: Added ISO 8601 time format output. #

Total comments: 2

Patch Set 6 : code review 179079: Added ISO 8601 time format output. #

Total comments: 5

Patch Set 7 : code review 179079: Added ISO 8601 time format output. #

Patch Set 8 : code review 179079: Added ISO 8601 time format output. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+43 lines, -1 line) Patch
M src/pkg/time/time.go View 1 2 3 4 5 3 chunks +22 lines, -1 line 0 comments Download
M src/pkg/time/time_test.go View 1 2 3 4 5 6 7 1 chunk +21 lines, -0 lines 0 comments Download

Messages

Total messages: 21
benolive
Hello golang-dev@googlegroups.com (cc: benolive, golang-dev@googlegroups.com), I'd like you to review the following change.
14 years, 3 months ago (2009-12-16 14:36:48 UTC) #1
r
http://codereview.appspot.com/179079/diff/1005/1006 File src/pkg/time/time.go (right): http://codereview.appspot.com/179079/diff/1005/1006#newcode323 src/pkg/time/time.go:323: decimal(buf[bp:bp+2], t.Month); delete all the trailing semicolons http://codereview.appspot.com/179079/diff/1005/1006#newcode386 src/pkg/time/time.go:386: ...
14 years, 3 months ago (2009-12-17 00:36:49 UTC) #2
rsc
> delete all the trailing semicolons cd $GOROOT/src hg sync all.bash hg gofmt
14 years, 3 months ago (2009-12-17 00:47:50 UTC) #3
benolive
>> delete all the trailing semicolons > > cd $GOROOT/src > hg sync > all.bash ...
14 years, 3 months ago (2009-12-17 00:56:17 UTC) #4
benolive
Removed the extraneous semicolons and changed that - to a comma http://codereview.appspot.com/179079/diff/1005/1006 File src/pkg/time/time.go (right): ...
14 years, 3 months ago (2009-12-17 00:58:25 UTC) #5
r2
looks good to me. russ? -rob
14 years, 3 months ago (2009-12-17 00:58:46 UTC) #6
rsc
http://codereview.appspot.com/179079/diff/9/1012 File src/pkg/time/time.go (right): http://codereview.appspot.com/179079/diff/9/1012#newcode363 src/pkg/time/time.go:363: func (t *Time) ISO8601() (formatted string) { s/(formatted string)/string/ ...
14 years, 3 months ago (2009-12-17 01:06:12 UTC) #7
benolive
Ok, I moved the timezone stuff to %z in format. I also noticed my code ...
14 years, 3 months ago (2009-12-17 01:28:17 UTC) #8
rsc
http://codereview.appspot.com/179079/diff/13/14 File src/pkg/time/time.go (right): http://codereview.appspot.com/179079/diff/13/14#newcode339 src/pkg/time/time.go:339: decimal(buf[bp:bp+2], int(-t.ZoneOffset/3600)) shouldn't need the int conversion here and ...
14 years, 3 months ago (2009-12-17 01:41:13 UTC) #9
benolive
I don't know why I did that. The cast didn't seem necessary so I removed ...
14 years, 3 months ago (2009-12-17 01:51:09 UTC) #10
rsc
Thanks. The code looks good; a couple nits on the tests. http://codereview.appspot.com/179079/diff/19/21 File src/pkg/time/time_test.go (right): ...
14 years, 3 months ago (2009-12-17 02:00:08 UTC) #11
benolive
http://codereview.appspot.com/179079/diff/19/21 File src/pkg/time/time_test.go (right): http://codereview.appspot.com/179079/diff/19/21#newcode104 src/pkg/time/time_test.go:104: func TestISO8601Conversion(t *testing.T) { On 2009/12/17 02:00:08, rsc wrote: ...
14 years, 3 months ago (2009-12-17 03:40:48 UTC) #12
rsc
> Ok, will do. Should I put the table at the top of the file ...
14 years, 3 months ago (2009-12-17 04:06:07 UTC) #13
benolive
http://codereview.appspot.com/179079/diff/19/21 File src/pkg/time/time_test.go (right): http://codereview.appspot.com/179079/diff/19/21#newcode104 src/pkg/time/time_test.go:104: func TestISO8601Conversion(t *testing.T) { On 2009/12/17 03:40:48, benolive wrote: ...
14 years, 3 months ago (2009-12-17 04:20:18 UTC) #14
rsc
almost there... http://codereview.appspot.com/179079/diff/19/21 File src/pkg/time/time_test.go (right): http://codereview.appspot.com/179079/diff/19/21#newcode104 src/pkg/time/time_test.go:104: func TestISO8601Conversion(t *testing.T) { On 2009/12/17 02:00:08, ...
14 years, 3 months ago (2009-12-17 04:27:12 UTC) #15
benolive
http://codereview.appspot.com/179079/diff/19/21 File src/pkg/time/time_test.go (right): http://codereview.appspot.com/179079/diff/19/21#newcode104 src/pkg/time/time_test.go:104: func TestISO8601Conversion(t *testing.T) { On 2009/12/17 04:27:12, rsc wrote: ...
14 years, 3 months ago (2009-12-17 04:46:01 UTC) #16
rsc
Code looks great. Thanks for the quick turnarounds. Final hurdle: you need to complete one ...
14 years, 3 months ago (2009-12-17 05:35:30 UTC) #17
benolive
I did that earlier today. Is there a delay? --Ben On Thu, Dec 17, 2009 ...
14 years, 3 months ago (2009-12-17 05:39:56 UTC) #18
rsc
> I did that earlier today. Is there a delay? No, sorry. I got faked ...
14 years, 3 months ago (2009-12-17 05:48:41 UTC) #19
rsc
LGTM Will submit after 180081, currently out for review.
14 years, 3 months ago (2009-12-17 20:25:19 UTC) #20
rsc
14 years, 3 months ago (2009-12-17 21:39:15 UTC) #21
*** Submitted as http://code.google.com/p/go/source/detail?r=feb2d8a970ac ***

time: add ISO 8601 time format

Fixes issue 431.

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

Committer: Russ Cox <rsc@golang.org>
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b