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

Switch to NPM and Update Dependency Libraries #21

Merged
merged 2 commits into from
Nov 29, 2022
Merged

Conversation

cmoesel
Copy link
Member

@cmoesel cmoesel commented Nov 28, 2022

Switch from yarn to NPM and update the dependency libraries to their latest versions.

Did not update node-fetch to 3.x because node-fetch 3.x does not support importing via CommonJS require statements (see README).

@cmoesel cmoesel requested a review from jafeltra November 28, 2022 22:01
@cmoesel
Copy link
Member Author

cmoesel commented Nov 29, 2022

If you'd like to test this in a CQL project, I've attached a zip here:
run-cql.zip

First, test the baseline (without this PR code):

  • Unzip it
  • cd into run-cql
  • Run npm install to install dependencies
  • Export the UMLS_API_KEY environment variable with your UMLS API key as the value
  • Run node .
  • You should see some data printed out. Ensure that some risk assessments are printed and that Fibromyalgia is printed (near the top).
  • Verify that there is a new .vscache folder with some XML files and a JSON file.

Then update the code to use this PR:

  • In this PR's source code, run npm install followed by npm pack to create a tgz package.
  • The rest of the steps should be done from the example CQL project (i.e., "run-cql")
  • Run npm install /your/path/to/cql-exec-vsac-1.2.2.tgz
  • Delete the .vsache folder that was created the first time you ran it.
  • Re-run node .
  • Verify that the printed out results are the same
  • Verify that .vscache is recreated with XML files and a JSON file

Copy link
Collaborator

@jafeltra jafeltra left a comment

Choose a reason for hiding this comment

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

This looks good to me! Thanks for providing the repo to test with. I tried it out and everything seems to work as expected still.

@cmoesel cmoesel merged commit c828ec4 into master Nov 29, 2022
@cmoesel cmoesel deleted the update-deps-nov-22 branch November 29, 2022 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants