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

3049 Use poco based parsers #3073

Merged
merged 11 commits into from
Mar 19, 2025
Merged

Conversation

ewoutkramer
Copy link
Member

@ewoutkramer ewoutkramer commented Mar 7, 2025

FIxes #3049.

This PR:

  • Moved the code from FhirXml/JsonPocoDeserializer to the existing original and old FhirXmlParser/FhirJsonParser.
  • It reuses ParserSettings, but adds the newer settings necessary for the newer POCO deserialization code
  • It re-instantes the old methods from FhirXmlParser as extension methods on the FhirXmlParser, so that it has both the newer POCO "deserialize" methods + the existing "Parse" methods.

The idea is that old code keeps compiling, but will start using the newer POCO code.

The biggest change is that the newer parsers are strict by default, so some unit tests (and FhirClient and DirectorySource) had to be changes to switch back to RECOVERABLE/OSTRICH.

@ewoutkramer ewoutkramer changed the base branch from develop to develop-6.0 March 8, 2025 06:34
Copy link
Member Author

@ewoutkramer ewoutkramer left a comment

Choose a reason for hiding this comment

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

Reviewed it for snafus, added come helpful comments for reviewer (I hope). No formal PR review.

@ewoutkramer ewoutkramer marked this pull request as ready for review March 10, 2025 14:08
@ewoutkramer ewoutkramer requested review from Kasdejong and andrzejskowronski and removed request for andrzejskowronski March 17, 2025 14:15
Copy link
Member

@Kasdejong Kasdejong left a comment

Choose a reason for hiding this comment

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

approved. One comment: will this not hugely interfere with andrzejs PR?

@ewoutkramer
Copy link
Member Author

ewoutkramer commented Mar 19, 2025

@ewoutkramer
[Merge branch 'develop-6.0' into 3049-use-poco-based-parsers](/FirelyTeam/firely-net-sdk/pull/3073/commits/aed8ef59b99f1e00437ec87a4fe631745d182788)

Verified
[aed8ef5](/FirelyTeam/firely-net-sdk/pull/3073/commits/aed8ef59b99f1e00437ec87a4fe631745d182788)

Yes, it will. Maybe I should not pull this. It seems it's easier to integrate his changes into my PR than my changes into his PR.

@ewoutkramer
Copy link
Member Author

Actually, no, because I did the renames in a separate project! So I am pulling this and will update his PR with develop6 afterwards.

@ewoutkramer ewoutkramer merged commit a99c6e0 into develop-6.0 Mar 19, 2025
16 checks passed
@ewoutkramer ewoutkramer deleted the 3049-use-poco-based-parsers branch March 19, 2025 13:22
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