Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Format #20

Merged
merged 6 commits into from
Sep 5, 2015
Merged

Format #20

merged 6 commits into from
Sep 5, 2015

Conversation

dcousens
Copy link
Member

@dcousens dcousens commented Sep 5, 2015

No description provided.

dcousens
dcousens reviewed Sep 5, 2015
View reviewed changes
sign.js
return new Buffer(out.toDER())
}
function dsaSign (hash, priv, algo) {
Copy link
Member Author

Choose a reason for hiding this comment

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

@calvinmetcalf are these meant to be here? They are completely unused.

@dcousens
Copy link
Member Author

dcousens commented Sep 5, 2015

Failing test, seems to be something wrong with parse-asn1?

/home/travis/build/crypto-browserify/browserify-sign/sign.js:23
  var len = priv.modulus.byteLength()
                         ^
TypeError: Cannot call method 'byteLength' of undefined

calvinmetcalf
calvinmetcalf reviewed Sep 5, 2015
View reviewed changes
sign.js
return ecSign(hash, priv)
} else if (priv.type === 'dsa') {
return dsaSign(hash, priv, hashType)
Copy link
Contributor

Choose a reason for hiding this comment

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

this is where dsaSign was used from

Copy link
Member Author

Choose a reason for hiding this comment

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

Cheers for re-adding it. Must have been lost when I re-based.

@calvinmetcalf
Copy link
Contributor

here we go with a version that passes standard and has DSA in it

@calvinmetcalf
Copy link
Contributor

everything passes, so merging

calvinmetcalf added a commit that referenced this pull request Sep 5, 2015
@calvinmetcalf
Merge pull request #20 from crypto-browserify/format
412eeef 
Format
@calvinmetcalf calvinmetcalf merged commit 412eeef into master Sep 5, 2015
@dcousens dcousens deleted the format branch September 5, 2015 03:50
@dcousens
Copy link
Member Author

dcousens commented Sep 5, 2015

Thanks @calvinmetcalf, not sure how that dsaSign got lost.

@calvinmetcalf
Copy link
Contributor

merge I'd guess, but that's why we use standard know

@calvinmetcalf
Copy link
Contributor

and published

# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers
No reviews
Assignees

@calvinmetcalf calvinmetcalf

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dcousens @calvinmetcalf