Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Issue #1446 - enable parsing dateTime fractional seconds up to 9 digits #1499

Merged
merged 4 commits into from
Sep 15, 2020
Merged

Issue #1446 - enable parsing dateTime fractional seconds up to 9 digits #1499

merged 4 commits into from
Sep 15, 2020

Conversation

michaelwschroeder
Copy link
Contributor

Signed-off-by: Mike Schroeder mschroed@us.ibm.com

@michaelwschroeder
Issue #1446 - enable parsing dateTime fractional seconds up to 9 digits
5c42c9e 
Signed-off-by: Mike Schroeder <mschroed@us.ibm.com>
prb112
prb112 reviewed Sep 10, 2020
View reviewed changes
Copy link
Contributor

@prb112 prb112 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also were there any previously disabled tests in fhir-persistence-jdbc?

please add a test in AbstractSearchDateTest

@prb112
Copy link
Contributor

prb112 commented Sep 10, 2020

Also were there any previously disabled tests in fhir-persistence-jdbc?

please add a test in AbstractSearchDateTest

Actually @michaelwschroeder that may not be so easy, if others think necessary, let's add, else skip.

@JohnTimm
Copy link
Collaborator

@michaelwschroeder where does the truncation happen?

@michaelwschroeder
Copy link
Contributor Author

@JohnTimm truncation happens in the DateTime and Instant constructors - this was already being done:

 value = ModelSupport.truncateTime(builder.value, ChronoUnit.MICROS);

@michaelwschroeder
Issue #1446 - update copyright
2227333 
Signed-off-by: Mike Schroeder <mschroed@us.ibm.com>
@michaelwschroeder
Issue #1446 - enable parsing Time fractional seconds up to 9 digits
432bb19 
Signed-off-by: Mike Schroeder <mschroed@us.ibm.com>
@JohnTimm
Copy link
Collaborator

@JohnTimm truncation happens in the DateTime and Instant constructors - this was already being done:

 value = ModelSupport.truncateTime(builder.value, ChronoUnit.MICROS);

Gotcha. I forgot that we added that. I thought that perhaps there was a truncate option in the DateTimeFormatter itself.

@JohnTimm JohnTimm requested a review from prb112 September 14, 2020 12:42
JohnTimm
JohnTimm approved these changes Sep 14, 2020
View reviewed changes
Copy link
Collaborator

@JohnTimm JohnTimm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

lmsurpre
lmsurpre reviewed Sep 14, 2020
View reviewed changes
fhir-model/src/main/java/com/ibm/fhir/model/type/DateTime.java Outdated
@@ -29,10 +29,13 @@
* A date, date-time or partial date (e.g. just year or year + month). If hours and minutes are specified, a time zone
* SHALL be populated. The format is a union of the schema types gYear, gYearMonth, date and dateTime. Seconds must be
* provided due to schema type constraints but may be zero-filled and may be ignored. Dates SHALL be valid dates.
* If seconds are specified, fractions of seconds may be specified up to nanosecond precision (9 digits). However, any
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pretty minor, but I'd like to see a <p> tag here. otherwise the javadoc gets consensed into one long paragraph and its not as clear whats from the spec vs from us

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

lmsurpre
lmsurpre reviewed Sep 14, 2020
View reviewed changes
fhir-model/src/main/java/com/ibm/fhir/model/type/Instant.java Outdated
@@ -23,10 +23,12 @@

/**
* An instant in time - known at least to the second
* Fractions of seconds may be specified up to nanosecond precision (9 digits). However, any fractions of seconds
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment, would like a <p> tag here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

lmsurpre
lmsurpre reviewed Sep 14, 2020
View reviewed changes
fhir-model/src/main/java/com/ibm/fhir/model/type/Time.java Outdated
import com.ibm.fhir.model.util.ValidationSupport;
import com.ibm.fhir.model.visitor.Visitor;

/**
* A time during the day, with no date specified
* Fractions of seconds may be specified up to nanosecond precision (9 digits). However, any fractions of seconds
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment, would like a <p> tag here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@michaelwschroeder
Issue #1446 - separate custom javadoc from spec with paragraph tag
66e7734 
Signed-off-by: Mike Schroeder <mschroed@us.ibm.com>
lmsurpre
lmsurpre approved these changes Sep 15, 2020
View reviewed changes
Copy link
Member

@lmsurpre lmsurpre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lmsurpre lmsurpre merged commit c039198 into master Sep 15, 2020
@lmsurpre lmsurpre deleted the issue-1446 branch September 15, 2020 02:24
lmsurpre added a commit that referenced this pull request Oct 19, 2020
@lmsurpre
issue #1499 - added docs
54c73f8 
Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this pull request Oct 19, 2020
@lmsurpre
issue #1499 - added docs
b955a3b 
Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this pull request Oct 19, 2020
@lmsurpre
issue #1499 - added docs
9c1e751 
Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this pull request Oct 19, 2020
@lmsurpre
issue #1499 - added docs
bf5f87e 
Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
@lmsurpre lmsurpre mentioned this pull request Nov 13, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lmsurpre lmsurpre lmsurpre approved these changes

@JohnTimm JohnTimm JohnTimm approved these changes

@prb112 prb112 Awaiting requested review from prb112

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@michaelwschroeder @prb112 @JohnTimm @lmsurpre