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

Use an MD5 hash instead of Object.hash for model hashes #147

Merged
merged 2 commits into from
Nov 22, 2024
Merged

Conversation

jakemac53
Copy link
Contributor

We can easily swap out the actual hash algorithm here now, in one place.

This is a bit slower (unsurprisingly) than using Object.hash etc, a few hundred milliseconds in total per iteration, but is a fairer representation of what we actually want.

@jakemac53
Copy link
Contributor Author

jakemac53 commented Nov 21, 2024

There is one failing test here - false and null generate the same digest. What value do you think we should use for the bytes of null/bool?

@jakemac53
Copy link
Contributor Author

jakemac53 commented Nov 21, 2024

We could include the type as a part of the digest for everything.... or maybe booleans get an extra byte, or we use const [1] and const [2] as for true/false when it comes to hashing, instead of 0 and 1.

Edit: I went with the [1] and [2] solution for false/true

@jakemac53 jakemac53 merged commit bf0ff2c into main Nov 22, 2024
51 checks passed
@jakemac53 jakemac53 deleted the use-md5 branch November 22, 2024 15:48
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