Issue 7069046: code review 7069046: time: standard time doc fix and clarification (Closed)
|
Created:
11 years, 3 months ago by cespare Modified:
11 years, 3 months ago Reviewers:
CC:
golang-dev, remyoudompheng, kevlar, rsc Visibility:
Public. |
Description time: standard time doc fix and format example
This fixes the incorrect unix timestamp of the standard time and adds
an example for (Time) Format to clarify how timezones work in format strings.
Fixes issue 4364.
Patch Set 1 #Patch Set 2 : diff -r 29fab86e961e https://code.google.com/p/go #Patch Set 3 : diff -r 29fab86e961e https://code.google.com/p/go #Patch Set 4 : diff -r 29fab86e961e https://code.google.com/p/go #
Total comments: 6
Patch Set 5 : diff -r c9c786583d50 https://code.google.com/p/go #Patch Set 6 : diff -r c9c786583d50 https://code.google.com/p/go #MessagesTotal messages: 16
Hello golang-dev@googlegroups.com (cc: rsc@golang.org), I'd like you to review this change to https://code.google.com/p/go
Sign in to reply to this message.
Please add "Fixes issue 4364" in the CL description.
Sign in to reply to this message.
Hello golang-dev@googlegroups.com, remyoudompheng@gmail.com (cc: golang-dev@googlegroups.com, rsc@golang.org), Please take another look.
Sign in to reply to this message.
https://codereview.appspot.com/7069046/diff/4002/src/pkg/time/format.go File src/pkg/time/format.go (right): https://codereview.appspot.com/7069046/diff/4002/src/pkg/time/format.go#newco... src/pkg/time/format.go:18: // includes a timezone, do not perform timezone conversions on the standard I don't understand this. How would you perform a timezone conversion on the standard time?
Sign in to reply to this message.
https://codereview.appspot.com/7069046/diff/4002/src/pkg/time/format.go File src/pkg/time/format.go (right): https://codereview.appspot.com/7069046/diff/4002/src/pkg/time/format.go#newco... src/pkg/time/format.go:18: // includes a timezone, do not perform timezone conversions on the standard You might think that 'writing what the standard time looks like formatted your way' would involve changing the hour of the standard time (3pm) from MST to, for example, UTC (if your format is intended to be a UTC time). See the issue for an example. This is probably worded poorly; maybe someone has an idea of how to phrase it more clearly and concisely. On 2013/01/07 19:23:48, kevlar wrote: > I don't understand this. How would you perform a timezone conversion on the > standard time?
Sign in to reply to this message.
https://codereview.appspot.com/7069046/diff/4002/src/pkg/time/format.go File src/pkg/time/format.go (right): https://codereview.appspot.com/7069046/diff/4002/src/pkg/time/format.go#newco... src/pkg/time/format.go:18: // includes a timezone, do not perform timezone conversions on the standard On 2013/01/07 19:33:40, cespare wrote: > You might think that 'writing what the standard time looks like formatted your > way' would involve changing the hour of the standard time (3pm) from MST to, for > example, UTC (if your format is intended to be a UTC time). See the issue for an > example. > > This is probably worded poorly; maybe someone has an idea of how to phrase it > more clearly and concisely. > > On 2013/01/07 19:23:48, kevlar wrote: > > I don't understand this. How would you perform a timezone conversion on the > > standard time? > I don't think the issue involves attempting timezone correction on the standard time, it stemmed from confusion about the discrepancy between the unix time and the given long-form time. The only thing that might bear adding is something that indicates that there are only a limited number of symbols accepted for variable representations of each field, but I'm not sure how to word that and I haven't really seen much confusion.
Sign in to reply to this message.
https://codereview.appspot.com/7069046/diff/4002/src/pkg/time/format.go File src/pkg/time/format.go (right): https://codereview.appspot.com/7069046/diff/4002/src/pkg/time/format.go#newco... src/pkg/time/format.go:18: // includes a timezone, do not perform timezone conversions on the standard Actually the confusion did involve attempting timezone correction on the standard time, but it was made worse by the unix time error. FWIW, I too was also initially tripped up when I first tried writing a time format. I tried converting the standard time to UTC but that doesn't work with (*Time) Format. The reason I was confused is that I thought the standard time was a canonical *instant in time*. On 2013/01/08 21:40:33, kevlar wrote: > On 2013/01/07 19:33:40, cespare wrote: > > You might think that 'writing what the standard time looks like formatted your > > way' would involve changing the hour of the standard time (3pm) from MST to, > for > > example, UTC (if your format is intended to be a UTC time). See the issue for > an > > example. > > > > This is probably worded poorly; maybe someone has an idea of how to phrase it > > more clearly and concisely. > > > > On 2013/01/07 19:23:48, kevlar wrote: > > > I don't understand this. How would you perform a timezone conversion on the > > > standard time? > > > > I don't think the issue involves attempting timezone correction on the standard > time, it stemmed from confusion about the discrepancy between the unix time and > the given long-form time. > > The only thing that might bear adding is something that indicates that there are > only a limited number of symbols accepted for variable representations of each > field, but I'm not sure how to word that and I haven't really seen much > confusion.
Sign in to reply to this message.
https://codereview.appspot.com/7069046/diff/4002/src/pkg/time/format.go File src/pkg/time/format.go (right): https://codereview.appspot.com/7069046/diff/4002/src/pkg/time/format.go#newco... src/pkg/time/format.go:18: // includes a timezone, do not perform timezone conversions on the standard On 2013/01/08 21:52:32, cespare wrote: > Actually the confusion did involve attempting timezone correction on the > standard time, but it was made worse by the unix time error. > > FWIW, I too was also initially tripped up when I first tried writing a time > format. I tried converting the standard time to UTC but that doesn't work with > (*Time) Format. > > The reason I was confused is that I thought the standard time was a canonical > *instant in time*. > > On 2013/01/08 21:40:33, kevlar wrote: > > On 2013/01/07 19:33:40, cespare wrote: > > > You might think that 'writing what the standard time looks like formatted > your > > > way' would involve changing the hour of the standard time (3pm) from MST to, > > for > > > example, UTC (if your format is intended to be a UTC time). See the issue > for > > an > > > example. > > > > > > This is probably worded poorly; maybe someone has an idea of how to phrase > it > > > more clearly and concisely. > > > > > > On 2013/01/07 19:23:48, kevlar wrote: > > > > I don't understand this. How would you perform a timezone conversion on > the > > > > standard time? > > > > > > > I don't think the issue involves attempting timezone correction on the > standard > > time, it stemmed from confusion about the discrepancy between the unix time > and > > the given long-form time. > > > > The only thing that might bear adding is something that indicates that there > are > > only a limited number of symbols accepted for variable representations of each > > field, but I'm not sure how to word that and I haven't really seen much > > confusion. > Ah, yes, I understand now. Perhaps the whole comment should be adjusted a bit: // These are predetermined layouts for use in Time.Format. // The standard time used in the layouts is: // ... // in which each part of the time corresponds to Unix time // xxxxxxxxx in America/Phoenix. Given that America/Phoenix // (MST above format) is GMT-7, this standard time can be // thought of as // ... // To define your own format, write down how the standard // time would look if each component of the standard time // were formatted to your liking. Note that the timezone // in this format is aesthetic and does not affect how // the hour is written. For examples of common formats // written in this way, see the values of ANSIC, StampMicro, // and Kitchen.
Sign in to reply to this message.
On 2013/01/08 22:16:25, kevlar wrote: > // These are predetermined layouts for use in Time.Format. > // The standard time used in the layouts is: > // ... > // in which each part of the time corresponds to Unix time > // xxxxxxxxx in America/Phoenix. Given that America/Phoenix > // (MST above format) is GMT-7, this standard time can be > // thought of as > // ... > // To define your own format, write down how the standard > // time would look if each component of the standard time > // were formatted to your liking. Note that the timezone > // in this format is aesthetic and does not affect how > // the hour is written. For examples of common formats > // written in this way, see the values of ANSIC, StampMicro, > // and Kitchen. proofreading fail. // These are predetermined layouts for use in Time.Format. // The standard time used in the layouts is: // ... // in which each part of the layout corresponds to Unix time // xxxxxxxxx in America/Phoenix. Given that America/Phoenix // (MST above) is GMT-7, this standard time can be // thought of as // ... // To define your own format, write down how the standard // time would look if each component of the standard time // were formatted to your liking. Note that the timezone // in this format is aesthetic and does not affect how // the hour is written. For examples of common formats // written in this way, see the values of ANSIC, StampMicro, // and Kitchen.
Sign in to reply to this message.
https://codereview.appspot.com/7069046/diff/4002/src/pkg/time/format.go File src/pkg/time/format.go (right): https://codereview.appspot.com/7069046/diff/4002/src/pkg/time/format.go#newco... src/pkg/time/format.go:18: // includes a timezone, do not perform timezone conversions on the standard I'm not happy with this either. I don't think the distinction between America/Phoenix and MST adds anything, and also I don't think that "aesthetic" is the right word. I like your placement of the sentence "Note that the timezone...", though, more than my change. What about this? // To define your own format, write down how the standard time // would look if each component were formatted to your liking. // (The hour is written the same way regardless of the timezone.) // For examples of common formats written in this way, see the // values of ANSIC, StampMicro, and Kitchen. On 2013/01/08 22:16:25, kevlar wrote: > On 2013/01/08 21:52:32, cespare wrote: > > Actually the confusion did involve attempting timezone correction on the > > standard time, but it was made worse by the unix time error. > > > > FWIW, I too was also initially tripped up when I first tried writing a time > > format. I tried converting the standard time to UTC but that doesn't work with > > (*Time) Format. > > > > The reason I was confused is that I thought the standard time was a canonical > > *instant in time*. > > > > On 2013/01/08 21:40:33, kevlar wrote: > > > On 2013/01/07 19:33:40, cespare wrote: > > > > You might think that 'writing what the standard time looks like formatted > > your > > > > way' would involve changing the hour of the standard time (3pm) from MST > to, > > > for > > > > example, UTC (if your format is intended to be a UTC time). See the issue > > for > > > an > > > > example. > > > > > > > > This is probably worded poorly; maybe someone has an idea of how to phrase > > it > > > > more clearly and concisely. > > > > > > > > On 2013/01/07 19:23:48, kevlar wrote: > > > > > I don't understand this. How would you perform a timezone conversion on > > the > > > > > standard time? > > > > > > > > > > I don't think the issue involves attempting timezone correction on the > > standard > > > time, it stemmed from confusion about the discrepancy between the unix time > > and > > > the given long-form time. > > > > > > The only thing that might bear adding is something that indicates that there > > are > > > only a limited number of symbols accepted for variable representations of > each > > > field, but I'm not sure how to word that and I haven't really seen much > > > confusion. > > > > Ah, yes, I understand now. Perhaps the whole comment should be adjusted a bit: > > // These are predetermined layouts for use in Time.Format. > // The standard time used in the layouts is: > // ... > // in which each part of the time corresponds to Unix time > // xxxxxxxxx in America/Phoenix. Given that America/Phoenix > // (MST above format) is GMT-7, this standard time can be > // thought of as > // ... > // To define your own format, write down how the standard > // time would look if each component of the standard time > // were formatted to your liking. Note that the timezone > // in this format is aesthetic and does not affect how > // the hour is written. For examples of common formats > // written in this way, see the values of ANSIC, StampMicro, > // and Kitchen.
Sign in to reply to this message.
Please fix the Unix time number and don't add other text. There's no agreement about what the text should say. Perhaps an example or two for Format, showing both UTC and not, would be useful.
Sign in to reply to this message.
(That is, a func ExampleTime_Format.)
Sign in to reply to this message.
PTAL. I removed the extra doc changes and added a Format example.
Sign in to reply to this message.
LGTM
Sign in to reply to this message.
*** Submitted as https://code.google.com/p/go/source/detail?r=86a58e67f6e6 *** time: standard time doc fix and format example This fixes the incorrect unix timestamp of the standard time and adds an example for (Time) Format to clarify how timezones work in format strings. Fixes issue 4364. R=golang-dev, remyoudompheng, kevlar, rsc CC=golang-dev https://codereview.appspot.com/7069046 Committer: Russ Cox <rsc@golang.org>
Sign in to reply to this message.
|
