Skip to content

EC keygen updates + changed ecdsa_sign to use BN_secure_new #8557

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.

Sign up for GitHub

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

Jump to bottom
Closed
slontis wants to merge 1 commit into from
Closed

EC keygen updates + changed ecdsa_sign to use BN_secure_new #8557

slontis wants to merge 1 commit into from

Conversation

@slontis
Copy link
Member

@slontis slontis commented Mar 22, 2019

Checklist
  • documentation is added or updated
  • tests are added or updated

romen
romen reviewed Mar 25, 2019
View reviewed changes
@romen romen self-assigned this Mar 25, 2019
@romen romen added the branch: master Applies to master branch label Mar 25, 2019
@romen romen mentioned this pull request Mar 25, 2019
Closed
2 tasks
@romen
Copy link
Member

romen commented Mar 27, 2019

@slontis , about force-pushing, in general I would recommend to avoid force-pushing to a PR under review as it makes it more difficult to trace what changed when.
In alternative you can just add fixup!/squash! commits during the review process to address the feedback, and we will take care of doing the fixup/squash during rebase when merging the PR in the development branch.

You can have a look at https://dev.to/koffeinfrei/the-git-fixup-workflow-386d (among the first results I got when googling about this) for more details and a better explanation of what I mean.

@slontis
Copy link
Member Author

slontis commented Mar 28, 2019

thanks for the tip. Wasnt aware of that feature.

romen
romen approved these changes Apr 3, 2019
View reviewed changes
Copy link
Member

@romen romen left a comment

Choose a reason for hiding this comment

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

LGTM

@romen romen added the approval: review pending This pull request needs review by a committer label Apr 3, 2019
@paulidale paulidale added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Apr 8, 2019
@romen
Copy link
Member

romen commented Apr 9, 2019

This has the necessary approvals, I will now proceed to merge!

@slontis
Copy link
Member Author

slontis commented Apr 9, 2019

@romen Thanks for all your contributions..

levitte pushed a commit that referenced this pull request Apr 9, 2019
@slontis @romen
EC keygen updates + changed ecdsa_sign to use BN_secure_new
bb315ca 
Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from #8557)
@romen
Copy link
Member

romen commented Apr 9, 2019

Merged on master with bb315ca

Thanks once again @slontis for your recent contributions!

@romen romen closed this Apr 9, 2019
@romen
Copy link
Member

romen commented Apr 9, 2019

@romen Thanks for all your contributions..

You ninja-ed me! :)

@dlegaultbbry dlegaultbbry mentioned this pull request Jun 11, 2019
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kroeckx kroeckx kroeckx left review comments

@paulidale paulidale paulidale approved these changes

+1 more reviewer

@romen romen romen approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@romen romen

Labels

approval: done This pull request has the required number of approvals branch: master Applies to master branch

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@slontis @romen @kroeckx @paulidale