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

Obsolete crc32 and use a internal copy #57

Closed
wants to merge 2 commits into from

Conversation

cptjazz
Copy link

@cptjazz cptjazz commented Jul 30, 2017

The type Crc32 is currently public, but it should not be.
The reasons are that this is a type that is only used as a "helper", and that this library is not dealing with hashing funtions per-se and hence should not publicly expose any.

Since we cannot delete (or internalise) a public type without introducing a breaking change, I marked the type as Obsolete to discourage its use by consumers of this library, and created an internal sealed class InternalCrc32 as internal replacement.

The original Crc32 can go out of service when the next breaking changes are introduced.

@JonathanMagnan JonathanMagnan self-assigned this Aug 1, 2017
@JonathanMagnan
Copy link
Member

Hello @cptjazz ,

Thank you for your contribution.

Best Regards,

Jonathan

@JonathanMagnan
Copy link
Member

Hello @cptjazz ,

Starting from v1.5.2-beta5, the Crc32 class has been marked obsolete.

There is currently a conflict now with this pull since all projects now use the shared project. You don't need to fix it.

We will add the Internal Crc32 class in a few weeks/months when we will believe it's a good time for a breaking changes.

Best Regards,

Jonathan

@JonathanMagnan
Copy link
Member

Hello @cptjazz ,

Starting from the version 1.5.6-beta1, the library is now CLSCompilant.

I will close this pull instead of merging it since I choosing to keep the same class name crc32 instead of adding the internal prefix.

Thank a lot for your contribution,

Best Regards,

Jonathan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants