fix: use -0700 when formatting time #1388
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sudoforge
force-pushed
the
Ifa198266c407524f7ef33ee33cf94ce9d0158f45
branch
2 times, most recently
from
07ea1eb to
9d52742
Compare
+0200 is not a valid reference identifier for the time format string, which requires a valid layout [0] using the reference time `01/02 03:04:05PM '06 -0700`. As the documentation notes: > It is a regrettable historic error that the date uses the American > convention of putting the numerical month before the day. This, combined with `-0700` being hardcoded into the layout requirements is what likely led to the confusion that caused this issue. This change is a fix for all `Time.Format()` calls, adjusting the time format in place to use the correct tzdata. As a future potential improvement, we should consider refactoring the format to use one of the constants in the time package that are exported for the different predefined formatting strings. This is not being done as part of this change because the current formatting string used in these calls does not match exactly with any of the predefined format strings. ... it isn't clear to me why this passes on CI. Using `+0200` to reference the timezone in the format string is invalid according to the `time` package documentation. [0]: https://pkg.go.dev/time#Layout Closes: #1387 Change-Id: Ifa198266c407524f7ef33ee33cf94ce9d0158f45
sudoforge
force-pushed
the
Ifa198266c407524f7ef33ee33cf94ce9d0158f45
branch
from
9d52742 to
2b41071
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
+0200 is not a valid reference identifier for the time format string,
which requires a valid layout 0 using the reference time
01/02 03:04:05PM '06 -0700.As the documentation notes:
This, combined with
-0700being hardcoded into the layout requirementsis what likely led to the confusion that caused this issue. This change
is a fix for all
Time.Format()calls, adjusting the time format inplace to use the correct tzdata. As a future potential improvement, we
should consider refactoring the format to use one of the constants in
the time package that are exported for the different predefined
formatting strings. This is not being done as part of this change
because the current formatting string used in these calls does not match
exactly with any of the predefined format strings.
... it isn't clear to me why this passes on CI. Using
+0200toreference the timezone in the format string is invalid according to the
timepackage documentation.Closes: #1387
Change-Id: Ifa198266c407524f7ef33ee33cf94ce9d0158f45