-
Notifications
You must be signed in to change notification settings - Fork 58
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
Implement function timezone()
#1386
Implement function timezone()
#1386
Conversation
Co-authored-by: Johannes Kalmbach <[email protected]>
Co-authored-by: Johannes Kalmbach <[email protected]>
Co-authored-by: Johannes Kalmbach <[email protected]>
Co-authored-by: Johannes Kalmbach <[email protected]>
…neral implementation
Co-authored-by: Johannes Kalmbach <[email protected]>
Co-authored-by: Johannes Kalmbach <[email protected]>
…ng correct prefix in Constants.h
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1386 +/- ##
==========================================
+ Coverage 89.06% 89.12% +0.06%
==========================================
Files 328 332 +4
Lines 29294 29494 +200
Branches 3262 3291 +29
==========================================
+ Hits 26090 26287 +197
- Misses 2054 2056 +2
- Partials 1150 1151 +1 ☔ View full report in Codecov by Sentry. |
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.
A first round of reviews.
I think we should fully integrate the
xs:dayTimeDuration
into the IDs.
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 already looks very mature.
I have made a thorough pass over everything, most things are small
(typos, documentation, readability).
Thank you very much.
test/DateYearDurationTest.cpp
Outdated
} | ||
|
||
//______________________________________________________________________________ | ||
DayTimeDuration::DurationValue toAndFromMilliseconds(int days, int hours, |
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 is the use case of 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.
W.r.t. large duration values, which are mostly internally normalized by conversion to milliseconds, it is quite complicated to test directly over the xsd:dayTimeduration strings, because the (input) duration string is different compared to the input string in terms of values. Thus i decided (while coding the setValues()
and getValues()
) to test this procedure explicitely.
Co-authored-by: Johannes Kalmbach <[email protected]>
Co-authored-by: Johannes Kalmbach <[email protected]>
Quality Gate passedIssues Measures |
timezone()
timezone()
xsd:DayTimeDuration
.xsd:DayTimeDuration
in general. They are stored inside the same class that also stores Date values, so they are folded directly into theValueID
.