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

Added support for seed in generating keypair #487

Merged
merged 5 commits into from Oct 20, 2018
Merged

Added support for seed in generating keypair #487

merged 5 commits into from Oct 20, 2018

Conversation

excerebrose
Copy link
Contributor

@excerebrose excerebrose commented Oct 19, 2018

Description

Adds support for seed in generating key pair as in the javascript driver, links to commit in the crypto conditions repository

Related PRs

Linked to change in cryptoconditions repo

Repo/Branch PR
Cryptoconditions/Master link

Todos

  • [ x ] Tested and working on development environment
  • [ x ] Added/Updated all related documentation. Add link if different from this PR

@ttmc
Copy link
Contributor

ttmc commented Oct 19, 2018

Hi @excerebrose --- See the comment I made on your related pull request in the cryptoconditions repository: bigchaindb/cryptoconditions#116 (comment)

Some tests are failing. There was already a docs test that was failing and we hadn't gotten around to fixing; don't worry about that one. Besides that, we'll also have to get your cryptodonditions pull request merged first, then release a new version of the cryptoconditions package, then modify this pull request to use the new cryptoconditions package. That should fix the other failing tests.

"""Generates a cryptographic key pair.

Args:
seed (str) : Optional Seed, defaults to `None`
Copy link
Contributor

Choose a reason for hiding this comment

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

This line of the docstring should be updated similar to the update suggested in bigchaindb/cryptoconditions#116 (comment)

@codecov-io
Copy link

Codecov Report

Merging #487 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #487   +/-   ##
=======================================
  Coverage   98.72%   98.72%           
=======================================
  Files           5        5           
  Lines         235      235           
=======================================
  Hits          232      232           
  Misses          3        3

@ttmc
Copy link
Contributor

ttmc commented Oct 20, 2018

@excerebrose I updated cryptoconditions to version 0.8.0 and fixed a small Flake8 error. There's still one test failing but it's a previously-known issue with the docs, which can be handled by another, future pull request (so don't worry about that). Please address my first comment about the docstring. (My second comment was wrong and I marked it as resolved.) Once that is done, I should be able to merge this pull request.

@excerebrose
Copy link
Contributor Author

@ttmc Fixed the docstring.you should be good to go

@ttmc ttmc merged commit 75b69c2 into bigchaindb:master Oct 20, 2018
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

3 participants