Automatically load hl7.fhir.uv.tools dependency #1186
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The current version of this package will always be loaded, even if it is not configured as a dependency. If it is configured as a dependency, use the configured version. When it fails to load, log a warning instead of an error if it was not a configured dependency.
cmoesel
requested changes
Dec 12, 2022
There was a problem hiding this comment.
This looks good and works well! I tested it locally w/ an existing cache as well as a cleared cache. I also tweaked the package date in the cached package and saw that it was detected as being stale. That's good!
I've only asked for one small change (to have the warning indicate it is an automatic dependency). The changes in the test file are all optional.
Add helper function to check for the presence of automatic dependencies in the package list.
jafeltra
approved these changes
Dec 13, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1166 and completes task CIMPL-1036.
The current version of this package will always be loaded, even if it is not configured as a dependency. If it is configured as a dependency, use the configured version. When it fails to load, log a warning instead of an error if it was not a configured dependency.
My strategy for implementing this was to consider loading dependencies in two groups: automatic dependencies and configured dependencies. Hopefully, this makes it straightforward to add more automatic dependencies later. There are changes to a lot of the tests for
loadExternalDependencies, since there's now an additional package that shows up every time. A lot of those tests are really testingloadConfiguredDependencies, though, so it may be worthwhile to adjust the tests accordingly. Please let me know if you think that would be useful.