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

#2901 Add support for overflow to POCOs #2918

Merged
merged 6 commits into from
Oct 18, 2024

Conversation

ewoutkramer
Copy link
Member

@ewoutkramer ewoutkramer commented Oct 16, 2024

Description

This PR is the first shot at adding support for overflow to POCOs. It should not introduce major breaking changes.

Remarks

⚠️ I had introduced the this[] operator for setting values, as we already had a getter for it hidden behind IReadOnlyDictionary. This meant that an existing this[] operator on the Parameter resource had to be removed as it conflicted with this operator in Base. Later I implemented this operator explicitly, so the problem does not exist anymore, but just for future flexibility I am removing this minor feature. This is a breaking change.

⚠️ Since pocos now implement IDictionary, they implement ICollection, which triggered bugs since we used a comparison to ICollection<T> to detect whether an element repeated or not. I changed the method in ReflectionHelper to detect IList<> instead. This is a breaking change.

⚠️ Also review and pull FirelyTeam/fhir-codegen#42, which are the changes to the codegen necessary to produce POCO's for this PR.

Related issues

Fixes #2901.

@ewoutkramer ewoutkramer changed the base branch from develop to develop-6.0 October 16, 2024 10:36
@ewoutkramer ewoutkramer requested a review from Kasdejong October 16, 2024 12:32
@ewoutkramer
Copy link
Member Author

ewoutkramer commented Oct 16, 2024

error CP0002: Member 'Hl7.Fhir.Model.Parameters.ParameterComponent Hl7.Fhir.Model.Parameters.this[string].get' exists on [Baseline] lib/net8.0/Hl7.Fhir.Base.dll but not on lib/net8.0/Hl7.Fhir.Base.dll [D:\a\1\s\src\Hl7.Fhir.Base\Hl7.Fhir.Base.csproj]

Use GetSingle() instead.

@ewoutkramer
Copy link
Member Author

error CP0002: Member 'bool Hl7.Fhir.Utility.ReflectionHelper.IsTypedCollection(System.Type)' exists on [Baseline] lib/net8.0/Hl7.Fhir.Base.dll but not on lib/net8.0/Hl7.Fhir.Base.dll [D:\a\1\s\src\Hl7.Fhir.Base\Hl7.Fhir.Base.csproj]

Use IsListCollection(). instead

@Kasdejong
Copy link
Member

There are too many things I want to comment on to do this via PR comments. Not because it's bad, but because I have not worked on this so I do not know what complications impacted your design :) We should discuss in person or over slack.

@ewoutkramer ewoutkramer merged commit 283ed30 into develop-6.0 Oct 18, 2024
16 checks passed
@ewoutkramer ewoutkramer deleted the 6.0/add-support-for-overflow branch October 18, 2024 13:34
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.

Add overflow Dictionary to POCOs
2 participants