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

Require minimal digest algorithm dependencies. #471

Merged
merged 2 commits into from
Jan 16, 2017
Merged

Require minimal digest algorithm dependencies. #471

merged 2 commits into from
Jan 16, 2017

Conversation

davidlehn
Copy link
Member

Trying to address concerns of #468

Unverified

No user is associated with the committer email.
- Improve custom builds by reducing module dependencies to only what is
  needed.
- Add `md.all.js` that includes all algorithms.
- Reduce dependencies to only required algorithms.
- Add some webpack examples.
@davidlehn davidlehn requested a review from dlongley January 16, 2017 20:05
@davidlehn
Copy link
Member Author

Note this could also be done to pbe.js. There is some code that can select different algorithms. It could change to not include all by default and throw an error if an algorithm is not found. Does it make sense in that file?

Is the md.all.js name ok?

@dlongley
Copy link
Member

It's fine to throw errors for people that are doing custom builds and trying to minimize included code (same goes for pbe). The name md.all.js is fine too. LGTM.

@dlongley
Copy link
Member

This is a related note that a significant amount of work has already been put into a new MessageDigest API and consolidation of common code in the 0.8.x branch. I just want to make sure we don't duplicate efforts at all (I don't think we are yet, just a warning):

https://github.com/digitalbazaar/forge/blob/0.8.x/js/md.js

- Custom builds should include digest algorithms they require.
- Throw errors as needed if algorithms do not exist.
@davidlehn
Copy link
Member Author

pbe.js updated now too. Will leave other use cases for specific requests or 0.8.x.

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.

None yet

2 participants