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

replace git-sha1 with simple-sha1 #236

Merged
merged 1 commit into from Jan 5, 2015
Merged

Conversation

@astro
Copy link
Contributor

astro commented Jan 5, 2015

See: #234

I would like to use the native crypto module in node.

contributes to GH issue #234
@feross

This comment has been minimized.

Copy link
Member

feross commented Jan 5, 2015

The native crypto module is already used in node. git-sha1 handles that case. See: https://github.com/creationix/git-sha1/blob/master/git-sha1.js#L11

I don't mind moving to simple-sha1, but first we need to confirm:

  • simple-sha1 (which uses Rusha internally) is actually faster than the git-sha1 implementation
  • the file size difference when browserified is negligible
  • swap out git-sha1 in all the webtorrent modules as it's used in several, and we don't want to include multiple implementations (I can handle this one)
@feross

This comment has been minimized.

Copy link
Member

feross commented Jan 5, 2015

Confirmed all 3 things. Thanks for the PR!

feross added a commit that referenced this pull request Jan 5, 2015
replace git-sha1 with simple-sha1
@feross feross merged commit 5ab99cc into webtorrent:master Jan 5, 2015
1 check passed
1 check passed
continuous-integration/travis-ci The Travis CI build passed
Details
@astro astro deleted the astro:simple-sha1 branch Jan 5, 2015
@lock lock bot locked as resolved and limited conversation to collaborators May 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.