-
Notifications
You must be signed in to change notification settings - Fork 126
examples: cleanups for better user experience #194
examples: cleanups for better user experience #194
Conversation
|
Hey @EmbeddedAndroid thanks for submitting this PR! I changed the target branch to Once the conflicts are resolved, I think this is ready to merge 😺 |
|
Thanks for reviewing, I'll get the conflicts sorted! |
0c3621a to
2d2a378
Compare
|
Patches should apply cleanly to the develop branch now! |
|
Sweet as. Cheers for that! Would you mind updating the formatting so that it matches PEP-8 (4-space indents, no spaces around Everything else looks good! |
2d2a378 to
c327e21
Compare
|
That should fix up the PEP-8 issues, let me know if you spot anything else. Cheers! |
todofixthis
left a comment
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.
Hey @EmbeddedAndroid I apologise for doing this, but I wanted to request two additional changes.
Those will be the last changes; everything else looks great!
|
|
||
| from iota import Iota, __version__ | ||
| from iota.crypto.addresses import AddressGenerator | ||
| from iota.crypto.types import Seed |
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.
❕ This import is used on line 60, as part of a type hint:
https://github.com/iotaledger/iota.lib.py/blob/develop/examples/address_generator.py#L59-L60
Even though the type hint is in a comment (for compatibility with Python 2), IDEs use the import to resolve the type hint correctly.
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.
Ah good catch, should have looked closed. Will drop this patch.
examples/send_transfer.py
Outdated
| help= | ||
| 'Users IOTA seed.' | ||
| '(defaults to THESEEDOFTHEWALLETSENDINGGOESHERE99999999999999999999999999999999999999999999999).', | ||
| ) |
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.
❔ I'm a tad bit uncomfortable having an interface for users to enter their seed in cleartext. Could we change this to use the functions from https://github.com/iotaledger/iota.lib.py/blob/develop/examples/address_generator.py#L46-L80 instead?
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.
Travelling at the moment, but will implement this as soon as I can.
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.
Cheers mate!
96e485f to
d9750eb
Compare
|
@todofixthis comments addressed finally :) |
Some notable changes: * Add argparse interface * Add hook for retrieving PyOTA library version * Clean up IOTA library imports * Ensure seed is not displayed in cleartext * Fix invalid depth * Misc PEP8 formatting fixes Signed-off-by: Tyler Baker <tyler.baker@iota.org>
d9750eb to
f072b2f
Compare
todofixthis
left a comment
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.
Looks good! Thanks @EmbeddedAndroid !!
…ransfer-example examples: cleanups for better user experience
Some notable changes: