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.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
std.time: add Date #18272
std.time: add Date #18272
Changes from 1 commit
4ab2fd9
9667506
4d09ffb
f227d1a
00530b1
ad8fdf6
9034ea1
f7b4297
ea54ee9
f1effb2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: could mention here 60 [leap second]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
regarding year_day and week_day - what's the reason to carry those around with any datetime instance? why not calculate them "on demand"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why carry anything but the timestamp and the timezone offset? Either way removing them doesn't affect the size of the struct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think Go takes the timestamp approach. Rust's Chrono seems to be in the middle. I feel that one benefit of a timestamp approach is that it's easier to implement date arithmetic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't mean to advocate a purely timestamp-based approach. If there is a need to have fields like
year_day
stored in the structure then that's fine. I was just thinking that fields that are rarely required could be replaced by methods that calculate the value on request for given datetime.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like these need to be off in some Locale specific area; not as a
pub
field in theMonth
enum.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these
names
constants be provided at all? They are specifically English and not localized and if you do not intend to localize your application to languages other than English it's not too difficult to do@tagName
on the tag and then customize the capitalization as well. Maybe I want to capitalize it "january" or "JANUARY" instead.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
60 [leap second]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about using
%f
for the fractional seconds part? That would make it independent of the resolution (ms, µs, ns).