-
-
Notifications
You must be signed in to change notification settings - Fork 13
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
feat: adding ssri.create for a crypto style interface #2
Conversation
adding crypto.create adding toJSON to Integrity
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.
Thanks a bunch for this PR! This is a really handy mechanism that I'm glad we're gonna have available <3
I added a few pretty minor nitpicks. Up to you whether you want to fix them, but it would be nice. :)
index.js
Outdated
function createIntegrity (opts) { | ||
opts = opts || {} | ||
|
||
opts = opts || {} |
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.
doubleplus options!
README.md
Outdated
Returns the string representation of an `Integrity` object. All hash entries | ||
will be concatenated in the string by `' '`. | ||
|
||
this is a convenience method so you can pass an `Integrity` object directly to `JSON.stringify` |
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.
Minor nit: capitalization
README.md
Outdated
will be concatenated in the string by `' '`. | ||
|
||
this is a convenience method so you can pass an `Integrity` object directly to `JSON.stringify` | ||
this works with the built in native JSON feature. For more info check out [toJSON() behavior on mdn](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#toJSON%28%29_behavior) |
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.
Minor nit: period at end of sentence?
README.md
Outdated
#### <a name="create"></a> `> ssri.create([opts]) -> <Integrity>` | ||
|
||
Returns an Integrity object calculated by reading data from | ||
calls to update |
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.
Minor nit: period missing.
create was misdocumented as returning Integrity but it return a Hash
=) feel free to merge when you are ready |
adding crypto.create
adding toJSON to Integrity