-
Notifications
You must be signed in to change notification settings - Fork 182
Fix: Conversion MedicationRequest 30-40 unmapped fields #2066
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
base: master
Are you sure you want to change the base?
Conversation
@@ -439,61 +436,4 @@ public void copyDomainResource(org.hl7.fhir.r4.model.DomainResource src, | |||
.map(Extension30_40::convertExtension) | |||
.forEach(tgt::addModifierExtension); | |||
} | |||
|
|||
public static org.hl7.fhir.r4.model.MedicationRequest convertMedicationRequest(org.hl7.fhir.dstu3.model.MedicationRequest src) throws FHIRException { |
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.
Moved to MedicationRequest30_40.java
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2066 +/- ##
============================================
+ Coverage 13.06% 13.07% +0.01%
- Complexity 35997 36044 +47
============================================
Files 2447 2447
Lines 722902 722917 +15
Branches 211376 211382 +6
============================================
+ Hits 94418 94543 +125
+ Misses 595845 595672 -173
- Partials 32639 32702 +63 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
91bc846
to
8153274
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.
See comments. The new test is excellent, as it gives us coverage for existing code, but it should also specifically target any added conversion logic. (I realize this is additional work, and do appreciate what has been done so far).
...s/src/main/java/org/hl7/fhir/convertors/conv30_40/resources30_40/MedicationRequest30_40.java
Show resolved
Hide resolved
...s/src/main/java/org/hl7/fhir/convertors/conv30_40/resources30_40/MedicationRequest30_40.java
Show resolved
Hide resolved
...s/src/main/java/org/hl7/fhir/convertors/conv30_40/resources30_40/MedicationRequest30_40.java
Show resolved
Hide resolved
...s/src/main/java/org/hl7/fhir/convertors/conv30_40/resources30_40/MedicationRequest30_40.java
Show resolved
Hide resolved
@dotasek can you have another look at this PR? All your remarks should have been processed |
org.hl7.fhir.convertors/src/test/resources/medication_request_30_converted_to_40.json
Outdated
Show resolved
Hide resolved
…y/requester/basedon)
5860625
to
aa20966
Compare
@dotasek @grahamegrieve could this get another review please 🙏 |
(Sorry for auto formatting, best to review with hide whitespace enabled)