Skip to content
This repository has been archived by the owner on Feb 22, 2019. It is now read-only.

Switch to hash.js from sha1 #67

Merged
merged 2 commits into from
Jul 20, 2018
Merged

Switch to hash.js from sha1 #67

merged 2 commits into from
Jul 20, 2018

Conversation

blowery
Copy link
Contributor

@blowery blowery commented Jul 17, 2018

hash.js, using just sha1 is 2.3k gzipped, while sha1 is 8k.

Switch to save some bytes and align with what calypso uses.

Hash.js pulling just sha1 is 2.3k gzipped, while sha1 is 8k. Switch to save some bytes and align with what calypso uses.
Copy link
Member

@sirreal sirreal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a win, tests are passing.

Updated usage appears to align with docs:

var sha512 = require('hash.js/lib/hash/sha/512');
sha512().update('abc').digest('hex');

Any specific ways we should be testing this? I'm really not familiar with i18n-calypso inner workings.

@blowery
Copy link
Contributor Author

blowery commented Jul 20, 2018

Testing... Yeah.

The existing unit tests sort of cover this, but there are no tests (that I can find) that explicitly cover the caching bit. I'll see if I can work something up.

@blowery
Copy link
Contributor Author

blowery commented Jul 20, 2018

@sirreal added tests for hashed lookups. They pass on master as well as here.

@blowery blowery requested a review from jsnajdr July 20, 2018 15:45
@blowery blowery self-assigned this Jul 20, 2018
@blowery blowery requested a review from flootr July 20, 2018 15:46
@blowery blowery merged commit e242706 into master Jul 20, 2018
@blowery blowery deleted the update/hash-library branch July 20, 2018 18:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants