-
Notifications
You must be signed in to change notification settings - Fork 176
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
DurationFormatter: Digital Data Provider SourceDataProvider impl #5208
DurationFormatter: Digital Data Provider SourceDataProvider impl #5208
Conversation
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.
Good job, add the result of cargo make testdata
Note that "durationUnit-type-hm": {
"durationUnitPattern": "h:mm"
},
"durationUnit-type-hms": {
"durationUnitPattern": "hh:mm:ss"
}, |
3ff08a8
to
49c508b
Compare
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.
Good code.
"m": 1, | ||
"s": 1 | ||
} | ||
} |
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.
this can be optimized, but we can do it in another PR: @kartva , just file a but to optimize this.
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.
To clarify, you are recommending that I file an issue about optimizing this?
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 @younies intended to say "file a bug", so yes. Alternatively, if you already have an issue where you are tracking follow-ups, you can add it there.
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.
This should be easy to bitpack: I think the only possible values are
pub enum HmsPadding {
None,
Hour,
Minute,
Second,
HourMinute,
HourSecond,
HourMinuteSecond,
}
Are there any locales with patterns such as the following? If not, we can cut down the options further:
h:m:ss
h:mm:s
h:m:s
m:s
Then I think the only options are
pub enum HmsPadding {
None,
/// Pad the hours in hh:mm and hh::mm:ss; pad the minutes in mm:ss
LargestUnit,
}
Will add bakeddata and testdata in separate PR.
Follow up to #5186.