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

Implemented STU3 #20

Merged
merged 5 commits into from
Jan 25, 2024
Merged

Implemented STU3 #20

merged 5 commits into from
Jan 25, 2024

Conversation

jarimayenburg
Copy link

Great library! We needed STU3 support so I used your generator to generate the STU3 model and added an STU3 client with the appropriate tests. All tests should be passing, let me know what you think.

@FlixCoder
Copy link
Owner

Thanks, I will need some time to look up the STU3 spec and such and check everything, as it is just my free time :D

@jarimayenburg
Copy link
Author

Take all the time you need, we will be using our fork until this is merged anyway

Copy link
Owner

@FlixCoder FlixCoder left a comment

Choose a reason for hiding this comment

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

Alright, found time quite early actually, so was way faster.

I still need to check a few things, but it already looks quite good! I mean much of it is just a copy (still need to find a way to get rid of the code duplication, any nice ideas?).

Only missing checks:

  • Completeness of test examples
  • Correctness of JSON schema
  • Generated code matches the current generate crate
  • xtask test actually runs through

So everything should be fine ^^

A few remarks I have:

crates/fhir-sdk/src/client/mod.rs Outdated Show resolved Hide resolved
crates/generate/Cargo.toml Show resolved Hide resolved
crates/generate/src/generate/gen_types.rs Outdated Show resolved Hide resolved
@FlixCoder
Copy link
Owner

Huh, apparently you aren't using the most up to date definitions of STU3. Do you mind if I switch to the newest downloads version? Do I have access to push here? :D I would of course only add a commit so your contribution would remain the same.

There is also (also done on my local branch):

  • Client needs a method stu3 to switch the client to STU3 mode.
  • OCD kicking in with STU3 not being in front of R4B in some cases :P
  • VSCode settings need to enable STU3 feature to show

@FlixCoder
Copy link
Owner

FlixCoder commented Jan 24, 2024

Ouch the new version does not include the code field, but only extensions (_code). This plays directly into a "bug" in this library and makes it very difficult to use.. I guess your version is fine

@jarimayenburg
Copy link
Author

jarimayenburg commented Jan 25, 2024

Huh, apparently you aren't using the most up to date definitions of STU3. Do you mind if I switch to the newest downloads version? Do I have access to push here? :D I would of course only add a commit so your contribution would remain the same.

The version of STU3 I used was 3.0.2, released on Feb 21, 2017. That should be the latest one right? Like you, I had to edit a bunch of things in the spec JSON files to get it working. Some things I changed:

  1. Added experimental, version and fhirVersion fields to StructureDefinitions and CodeSystems wherever they were missing.
  2. Added code fields to ElementDefinitions of primitive datatypes that only had a _code extension.
  • Client needs a method stu3 to switch the client to STU3 mode.

Let me add that!

  • OCD kicking in with STU3 not being in front of R4B in some cases :P

Haha I had the same OCD kicking in. Problem is that rustfmt orders modules alphabetically, so stu3 will land after r4b and r5. Feel free to order it however you'd like when it's merged though.

  • VSCode settings need to enable STU3 feature to show

Whoops, I don't use VSCode so I missed that. Added it now.

@FlixCoder
Copy link
Owner

he version of STU3 I used was 3.0.2, released on Feb 21, 2017. That should be the latest one right?

Hmm the version.info file says it is 3.0.1 :D And the newest version is a build of 3.0.2 from 2019 apparently.

Haha I had the same OCD kicking in. Problem is that rustfmt orders modules alphabetically, so stu3 will land after r4b and r5. Feel free to order it however you'd like when it's merged though.

No I am using rustfmt too ^^ So I will need to live with it :D

Alright, I think that's it, I need to take a final look and then I will merge it, thank you very much!

@jarimayenburg
Copy link
Author

jarimayenburg commented Jan 25, 2024

he version of STU3 I used was 3.0.2, released on Feb 21, 2017. That should be the latest one right?

Hmm the version.info file says it is 3.0.1 :D And the newest version is a build of 3.0.2 from 2019 apparently.

On the version history page (https://hl7.org/fhir/history.html), in that big table, I think the correct latest version is called "3.0.0" and released on Mar 21, 2017. While at the top of the page they refer to Release 3 STU as 3.0.2, released on Feb 21, 2017. Not sure how a newer version 3.0.2 has an earlier release date than the older version 3.0.0, but here we are haha.

Anyway, I used the link to 3.0.2 in the "major milestones" to get to the final STU3 spec and got the specification from the download page of that version: https://hl7.org/fhir/stu3/downloads.html. I can't imagine that that is an older version.

The version.info file came from there as well. I have no idea why that file is then suddenly referring to a version 3.0.1, but it's all pretty confusing. Feel free to change it if you want.

@FlixCoder
Copy link
Owner

Alright, then it should be the newest :D Weird. No, totally fine then, thanks!

@FlixCoder FlixCoder merged commit 28e3603 into FlixCoder:main Jan 25, 2024
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