Skip to content

Refactor to actually be useful as a library#93

Merged
achow101 merged 2 commits into
masterfrom
librarify
Jan 21, 2019
Merged

Refactor to actually be useful as a library#93
achow101 merged 2 commits into
masterfrom
librarify

Conversation

@achow101
Copy link
Copy Markdown
Member

@achow101 achow101 commented Jan 6, 2019

Refactors command.py so that it just has the functions for each command and the arguments are the actual arguments for the command instead of args and client. process_commands has been moved to cli.py along with the handlers that take args and client which then call the functions in commands.py to do the command.

setup.py has been updated to also install a hwi executable script and the version number is bumped to 0.0.5.

This makes HWI actually usable as a library while also having the command line functionality.

Completes part 1 of #91

@achow101
Copy link
Copy Markdown
Member Author

Had to rebase

@instagibbs
Copy link
Copy Markdown
Collaborator

failing:

0.36s$ cd test; ./run_tests.py
Traceback (most recent call last):
  File "./run_tests.py", line 13, in <module>
    from test_ledger import ledger_test_suite
  File "/home/travis/build/achow101/HWI/test/test_ledger.py", line 15, in <module>
    from hwilib.commands import process_commands
ImportError: cannot import name 'process_commands'

@achow101
Copy link
Copy Markdown
Member Author

Fixed the error

@achow101 achow101 force-pushed the librarify branch 2 times, most recently from 13ed96f to f94fe58 Compare January 14, 2019 22:54
@Sjors
Copy link
Copy Markdown
Member

Sjors commented Jan 18, 2019

Concept ACK. But let's use named arguments, especially for getkeypool(client, args.path, args.start, args.end, args.internal, args.keypool, args.account, args.sh_wpkh, args.wpkh). That makes it easier for me in #73 to pass in a descriptor, without having to explicitly set the positional args.path, args.sh_wpkh, etc to None.

@achow101
Copy link
Copy Markdown
Member Author

Do you mean give each argument a default value or do you mean having the call be getkeypool(client=client, path=args.path, start=args.start, ...)?

@Sjors
Copy link
Copy Markdown
Member

Sjors commented Jan 18, 2019

The latter. It's probably fine to keep some arguments like client positional.

Moves the command argument parsing to hwi.py. Instead of each function
taking args and client, they now take client and whatever else they
need. hwi.py handles the conversion from args and client to the actual
arguments.
@achow101
Copy link
Copy Markdown
Member Author

@Sjors Done.

@Sjors
Copy link
Copy Markdown
Member

Sjors commented Jan 21, 2019

Travis unhappy:

error: click 6.7 is installed but click<8,>=7 is required by {'trezor'}
The command "python setup.py install" failed and exited with 1 during .

@achow101
Copy link
Copy Markdown
Member Author

Seems like a spurious failure. I restarted the travis build and it seems fine now

@Sjors
Copy link
Copy Markdown
Member

Sjors commented Jan 21, 2019

tACK 8e4b3fc (based on testing #73, which I just rebased)

@achow101 achow101 merged commit 8e4b3fc into master Jan 21, 2019
achow101 added a commit that referenced this pull request Jan 21, 2019
8e4b3fc Add executable script to setup.py and bump version number (Andrew Chow)
2ddd4ed Refactor to make hwilib more like a library (Andrew Chow)

Pull request description:

  Refactors `command.py` so that it just has the functions for each command and the arguments are the actual arguments for the command instead of `args` and `client`. `process_commands` has been moved to `cli.py` along with the handlers that take `args` and `client` which then call the functions in `commands.py` to do the command.

  `setup.py` has been updated to also install a `hwi` executable script and the version number is bumped to 0.0.5.

  This makes HWI actually usable as a library while also having the command line functionality.

  Completes part 1 of #91

Tree-SHA512: 73c36f79b3f28140f1600cd263325d2fffc422b759884bccb613f0ca922d76801973a41fbd52cba0b4142f19f97ac465cfe7277748e4511cadc461f16ba3026c
@achow101 achow101 deleted the librarify branch January 21, 2019 19:03
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.

3 participants