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.

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

Moved the NpmPackage dependency for structure-map based resource extraction. #844

Merged
merged 6 commits into from
Nov 10, 2021

Conversation

aditya-07
Copy link
Collaborator

@aditya-07 aditya-07 commented Oct 19, 2021

IMPORTANT: All PRs must be linked to an issue (except for extremely trivial and straightforward changes).

Fixes #808

Description
Moved the NpmPackage dependency for structure-map based resource extraction action down from datacapture library to the client apps. Now the onus is on the client apps to load the NpmPackage with all the resources that maybe required by the structure-map.

This has reduced the size of our Reference App from 48.8 MB to 35.6 MB, packages.fhir.org-hl7.fhir.r4.core-4.0.1.tgz being 12.8MB.
Alternative(s) considered
No

Type
Choose one: Code health

Screenshots (if applicable)

Checklist

  • I have read and acknowledged the Code of conduct
  • I have read How to Contribute
  • I have read the Developer's guide
  • I have signed the Google Individual CLA, or I am covered by my company's Corporate CLA
  • I have discussed my proposed solution with code owners in the linked issue(s) and we have agreed upon the general approach
  • I have run ./gradlew spotlessApply and ./gradlew spotlessCheck to check my code follows the style guide of this project
  • I have run ./gradlew check and ./gradlew connectedCheck to test my changes locally
  • I have built and run the reference app(s) to verify my change fixes the issue and/or does not break the reference app(s)

…action down from datacapture library to the client apps.
Copy link
Collaborator

@jingtang10 jingtang10 left a comment

Choose a reason for hiding this comment

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

thanks @aditya-07

@ekigamba FYI - we needed to remove the npm package because it bloats the library. but if the client does need to load a custom npm package they can provide it using the config. and that's a functionality (e.g. TarGzipUtility.kt and NpmPackageProvider.kt) that may well exist in fhir core lib. @pld FYI as well.

@jingtang10
Copy link
Collaborator

jingtang10 commented Oct 23, 2021

btw - have you noticed the size decrease before vs after this PR :) can you share the difference?

@jingtang10
Copy link
Collaborator

btw - have you noticed the size decrease before vs after this PR :) can you share the difference?

it's already in the pr description. thanks for pointing out.

@codecov
Copy link

codecov bot commented Nov 2, 2021

Codecov Report

Merging #844 (db82cd2) into master (7323e5c) will increase coverage by 0.17%.
The diff coverage is 70.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #844      +/-   ##
============================================
+ Coverage     33.36%   33.53%   +0.17%     
+ Complexity      458      457       -1     
============================================
  Files           108      105       -3     
  Lines          9033     8981      -52     
  Branches        560      552       -8     
============================================
- Hits           3014     3012       -2     
+ Misses         5732     5684      -48     
+ Partials        287      285       -2     
Impacted Files Coverage Δ
...ogle/android/fhir/datacapture/DataCaptureConfig.kt 75.00% <66.66%> (-25.00%) ⬇️
...android/fhir/datacapture/mapping/ResourceMapper.kt 80.90% <71.42%> (-0.10%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7323e5c...db82cd2. Read the comment docs.

@jingtang10 jingtang10 enabled auto-merge (squash) November 10, 2021 19:33
@jingtang10 jingtang10 merged commit e31b492 into master Nov 10, 2021
@jingtang10 jingtang10 deleted the ak/move-npmpackage-dependency branch November 10, 2021 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Remove NpmPackage dependency from datacapture library. Data capture gallery app crash on pre-24 emulators
3 participants