Skip to content

Conversation

@mmsmits
Copy link
Member

@mmsmits mmsmits commented Oct 8, 2024

fixes #1814

@mmsmits mmsmits marked this pull request as ready for review October 8, 2024 09:34
@mmsmits mmsmits requested a review from ewoutkramer October 8, 2024 09:35
@mmsmits mmsmits enabled auto-merge October 8, 2024 09:35
// Also ignore any Changed extensions on base and diff
elemClone.RemoveAllConstrainedByDiffExtensions();
baseClone.RemoveAllConstrainedByDiffExtensions();
elemClone.RemoveAllNonInheritableExtensions();
Copy link
Member

Choose a reason for hiding this comment

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

Does this method replace the original ones? So RemoveallNonInheritableExtension replaces RemoveAllConstrainedByDiffExtensions? If so, should we not remove/obsolete the original method?

Copy link
Member Author

@mmsmits mmsmits Oct 21, 2024

Choose a reason for hiding this comment

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

Well, I thought about that, but didn't dare to since Forge might be using the ConstraintByDiff extension one maybe?
And that is a bit faster (I don't know if it might be noticeable in a UI).
@Rob5045 any feedback here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Forge does not use the RemoveAllConstrainedByDiffExtensions extension.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, I'll Obsolete is then

Copy link
Contributor

@Rob5045 Rob5045 Oct 21, 2024

Choose a reason for hiding this comment

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

I'm working on supporting extensions on element definition properties in Forge. That would mean that the UI won't be cluttered with these I guess? That would be nice.
image

Copy link
Contributor

@Rob5045 Rob5045 Oct 21, 2024

Choose a reason for hiding this comment

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

How about http://hl7.org/fhir/StructureDefinition/elementdefinition-bindingName? I see that one all the time as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

@mmsmits mmsmits requested a review from ewoutkramer October 22, 2024 13:57
mmsmits and others added 3 commits October 22, 2024 16:57
@mmsmits mmsmits requested a review from ewoutkramer October 22, 2024 14:58
@mmsmits mmsmits merged commit bbf6526 into develop Oct 23, 2024
16 checks passed
@mmsmits mmsmits deleted the feature/1814-remove-all-non-inheritable-extensions-from-base branch October 23, 2024 08:13
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.

Make sure we do not propagate "publishing extensions" while creating a snapshot

4 participants