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

Changes coming from Shim PR and running the original Tests #205

Merged
merged 9 commits into from
Nov 20, 2023

Conversation

mmsmits
Copy link
Member

@mmsmits mmsmits commented Nov 13, 2023

No description provided.

mmsmits and others added 6 commits November 10, 2023 14:39
# Conflicts:
#	src/Firely.Fhir.Validation/Impl/DatatypeSchema.cs
#	src/Firely.Fhir.Validation/Impl/ExtensionSchema.cs
#	src/Firely.Fhir.Validation/Impl/PatternValidator.cs
#	src/Firely.Fhir.Validation/Impl/ReferencedInstanceValidator.cs
#	src/Firely.Fhir.Validation/Impl/ResourceSchema.cs
#	src/Firely.Fhir.Validation/Impl/SchemaReferenceValidator.cs
#	src/Firely.Fhir.Validation/Impl/SliceValidator.cs
#	src/Firely.Fhir.Validation/Support/FhirSchemaGroupAnalyzer.cs
#	test/Firely.Fhir.Validation.Compilation.Tests.Shared/FhirTestCases
@mmsmits mmsmits requested a review from ewoutkramer November 14, 2023 08:47
Copy link
Member

@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.

Great work. Just one minor question.

/// Returns the slice name of the last slice in the path, or an empty string if there is no slice information.
/// </summary>
/// <returns>The slice name of the last slice in the path, or an empty string if there is no slice information.</returns>
public string GetSliceInfo()
Copy link
Member

Choose a reason for hiding this comment

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

I hardly ever find returning empty strings useful, I think most people would expect null if there is no sensible slice info, not an empty string. Or, if you want to make it explicit, call the method TryGetSliceInfo().

@mmsmits mmsmits enabled auto-merge November 15, 2023 10:56
@mmsmits mmsmits requested a review from ewoutkramer November 15, 2023 10:56
@mmsmits mmsmits merged commit c056b5a into develop Nov 20, 2023
2 checks passed
@mmsmits mmsmits deleted the VONK-5092-Finish-the-Validator-Shim-PR branch November 20, 2023 17:03
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