FirelyTeam / firely-net-sdk

The official Firely .NET SDK for HL7 FHIR
Other
829 stars 345 forks source link

Inconsistent internal/public access for the SemVersion class #2420

Closed brianpos closed 1 year ago

brianpos commented 1 year ago

Describe the bug I was using this SemVersion class in my Facade project (in place of bringing in a dependency to an external package) which has now been marked as internal for netcore based projects (but still public for net452) https://github.com/FirelyTeam/firely-net-sdk/blob/7d63b1e1b2eafa9ff1d53c98f87cac409bd1f5e6/src/Hl7.Fhir.Base/Utility/SemVersion.cs#L16-L21 Can this be made public again?

Version used:

https://github.com/brianpos/fhir-net-web-api/blob/feature/r4b/src/Hl7.Fhir.WebApi.Support/CurrentCanonical.cs#L149-L153

mmsmits commented 1 year ago

SemVer is a class we copied from an external library, something we don't want to maintain for the community.

We made it internal to encourage the community to use an external nuget package instead like:

https://www.nuget.org/packages/Semver

Or

https://www.nuget.org/packages/SemanticVersion

mmsmits commented 1 year ago

And we don't support net452 anymore, as of 5.0.0. We should have removed that "if" here, that only causes confusion.

brianpos commented 1 year ago

Any reason why you don't use the external library then? A shame that different parts in an install will use different engines.

mmsmits commented 1 year ago

We don't want to add an extra dependency to the SDK, and leave the flexibility of choosing a SemVer library to our users.