Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

feat: Add ImplementationGuides compile method #38

Merged
merged 5 commits into from
Jan 29, 2021
Merged

Conversation

carvantes
Copy link
Contributor

Description of changes:

Mostly just refactoring the old .js compiler script into .ts to use it as the implementation of the ImplementationGuides interface compile method. The functionality of the compiler remains unchanged.

We are still bundling the FHIR R4 resources into the search package, so we retain scripts/searchParamsCompiler/run.ts as a convenient way to regenerate our schema files if we make changes the grammar/compiler.

Testing:
re-compiled the search params for fhir 4.0.1 and 3.0.1. There were no changes to the output, as expected.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

nguyen102
nguyen102 previously approved these changes Jan 25, 2021
Copy link
Contributor

@nguyen102 nguyen102 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, one nitpick about type definition for UNSUPPORTED_SEARCH_PARAMS. Also remember to update the version of the interface package. Looks like GH action Unit Tests and Linting threw an error for it.

);
};

const UNSUPPORTED_SEARCH_PARAMS = [
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be initialized as an array of string?

const UNSUPPORTED_SEARCH_PARAMS: string[]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doing so won't change the "initialization" of the variable. The difference would be declaring the type explicitly instead of letting TS infer it.

In many cases declaring types explicitly makes code more readable, but declaring types explicitly all the time can be overly verbose and is sort of a waste of the type inference capabilities of TS.

IMO here it's obvious that it is an array of strings since it's a small, fixed, array declared on that same line.

@carvantes carvantes requested a review from nguyen102 January 28, 2021 23:26
src/implementationGuides/index.ts Show resolved Hide resolved
src/implementationGuides/index.ts Show resolved Hide resolved
src/implementationGuides/index.ts Show resolved Hide resolved
src/implementationGuides/index.ts Outdated Show resolved Hide resolved
@carvantes carvantes requested a review from rsmayda January 29, 2021 08:44
@carvantes carvantes merged commit e0024a4 into mainline Jan 29, 2021
@carvantes carvantes deleted the dev-igs-compiler branch January 29, 2021 16:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants