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

Changed SupportedAlgorithms class to Registry Design Pattern. #21

Merged
merged 1 commit into from
Sep 26, 2019

Conversation

arabasso
Copy link
Contributor

Hello!

I am sending this modification to provide the possibility to alter the hashing and checksum algorithms of Octodiff (this will allow to use mainly more performative hashing algorithms).

@CLAassistant
Copy link

CLAassistant commented Jan 27, 2019

CLA assistant check
All committers have signed the CLA.

@droyad
Copy link
Contributor

droyad commented Sep 26, 2019

@arabasso Sorry, this slipped under the radar as the CLA was not signed. This looks good to me and I'm happy to merge it once the CLA is signed.

I couldn't think of a good way to keep this backwards compatible, so we'll need to bump the major

@arabasso
Copy link
Contributor Author

Hello! Sorry, just now I became aware of this CLA... Anyway, about the modification, I made it so that the code was fully backwards compatible.

@droyad droyad merged commit 7a7926c into OctopusDeploy:master Sep 26, 2019
@droyad
Copy link
Contributor

droyad commented Sep 26, 2019

@arabasso Thanks. It's compile time compatible, but I'm not sure it's runtime compatible. I might just check on that.

@droyad
Copy link
Contributor

droyad commented Sep 26, 2019

Yeh I get a System.TypeLoadException: Could not load type 'Checksum' from assembly 'Octodiff when I drop the the updated DLL into it. Only a problem if it on the crazy chance someone is calling those methods from outside.

@arabasso
Copy link
Contributor Author

Yes, it is not runtime compatible because additional classes and interfaces have been created. Anyone who wants to use this version of the library will have to recompile the project that uses it.

@arabasso
Copy link
Contributor Author

Also, it would be interesting to use the xxHash algorithm instead of SHA1 (SHA1 has an average speed of 0.28 GB/s, while xxHash has 5.4 GB/s).

https://cyan4973.github.io/xxHash/

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.

3 participants