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

Update dependencies and create DEPENDENCY-NOTES.md #265

Merged
merged 3 commits into from
Sep 5, 2024

Conversation

mint-thompson
Copy link
Collaborator

Description:
Dependencies are updated to their latest versions. In cases where they are not being updated to their latest versions, add a note to DEPENDENCY-NOTES.md explaining why it is not being updated.

Due to changes in the eslint dependency, create an eslint config file. Remove the deprecated .eslintignore and .eslintrc files.

Update jest config to use transform property instead of deprecated globals property. Create test/tsconfig.json to use with updated jest config.

Update tsconfig to target ES2018 to satisfy updated typescript compiler.

Minor changes to usage of commander package in app.ts.

Other formatting and style changes in various typescript files in order to satisfy latest eslint rules.

Testing Instructions:
Update all installed dependencies. Confirm that tests pass and that eslint/prettier report no problems. Run GoFSH on FHIR resources to make sure that dependency updates are not causing any problems that only appear at runtime.

Related Issue:
Fixes #263

mint-thompson and others added 2 commits August 27, 2024 14:25
Dependencies are updated to their latest versions. In cases where they
are not being updated to their latest versions, add a note to
DEPENDENCY-NOTES.md explaining why it is not being updated.

Due to changes in the eslint dependency, create an eslint config file.
Remove the deprecated .eslintignore and .eslintrc files.

Update jest config to use transform property instead of deprecated
globals property. Create test/tsconfig.json to use with updated jest
config.

Update tsconfig to target ES2018 to satisfy updated typescript compiler.

Minor changes to usage of commander package in app.ts.

Other formatting and style changes in various typescript files in order
to satisfy latest eslint rules.
Copy link
Member

@cmoesel cmoesel left a comment

Choose a reason for hiding this comment

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

When I ran npm install, I got a bunch of warnings like this:

npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE package: '@eslint/config-array@0.18.0',
npm WARN EBADENGINE required: { node: '^18.18.0 || ^20.9.0 || >=21.1.0' },
npm WARN EBADENGINE current: { node: 'v18.16.0', npm: '9.5.1' }
npm WARN EBADENGINE }

I updated the .tool-versions file to use a later version of Node 18 and that fixed it. I went ahead and pushed the change because I'm not sure if you use asdf.

I also left one comment about yaml for your consideration.

I ran this branch against US Core and compared it to output from master and there were no changes.

I'm not marking this approved or request changes -- as I want to see what your response is to that comment.

- `@types/node`: don't update until Node 22 is LTS version (currently Node 20).
- `chalk`: major version 5 causes problems for jest. Keep updated to latest 4.x release.
- `flat`: major version 6 is an esmodule.
- `yaml`: changes to `Document.toString()` behavior makes the comments in the config file produced by `sushi init` move around a bunch.
Copy link
Member

Choose a reason for hiding this comment

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

We don't use yaml for sushi init in GoFSH, do we? Assuming GoFSH does not rewrite the config, it seems to me we could update yaml.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's a good point! I'll see about updating it, then.

Changes to how the yaml package outputs comments do not affect the usage
of this package by ExportableConfiguration.
Copy link
Member

@cmoesel cmoesel left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks, Mint!

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.

All the changes make sense to me. I left one question about the DEPENDENCY-NOTES file itself.

DEPENDENCY-NOTES.md Show resolved Hide resolved
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.

Looks good.

@mint-thompson mint-thompson merged commit 0f0a5b1 into master Sep 5, 2024
14 checks passed
@mint-thompson mint-thompson deleted the cimpl-1272-update-dependencies branch September 5, 2024 13:27
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.

Update Dependencies and Add DEPENDENCY-NOTES.md
3 participants