-
Notifications
You must be signed in to change notification settings - Fork 1
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
Fixes for bugs found during review #3
base: master
Are you sure you want to change the base?
Conversation
@@ -1,7 +1,7 @@ | |||
{ | |||
"name": "cubehash.js", | |||
"description": "CubeHash is a very simple cryptographic hash function", | |||
"version": "1.0.0", | |||
"version": "1.0.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never make version changes on branches. When merged into master use npm version patch
to get the sae functionality, but also have the version bump in a separate commit with a git tag as well
Hash changes based on a change in the implementation point at a major version increase when this gets merged, since the changes are not backwards compatible |
I'd really like if we could improve the test suite to reflect all of these findings. We're just asserting on the resulting outcome instead of testing the mechanics of the algorithm, thus making the test useless in testing the actual specifications |
374c539
to
dd28b17
Compare
It would be nice to have proper tests of the internals. I propose merging this in, bump the version (I don't really care if it is a minor or major version) and then start a develop branch where we start with rewriting to make tests and maybe implement a more general version. The constants used are more or less grabbed from the thin air as it is right now. |
dd28b17
to
7ccec76
Compare
6427e27
to
7a46179
Compare
7a46179
to
917d5f9
Compare
Bugs described in #2