multi: add accounts screen #94
Conversation
buck54321
force-pushed
the
accountsui
branch
2 times, most recently
from
6ecb6da
to
2aef091
Compare
decred/decred/util/chains.py
Outdated
| @@ -93,5 +93,4 @@ def chain(coinType): | |||
| Returns: | |||
| obj: Network parameters. | |||
| """ | |||
| coinType = parseCoinType(coinType) | |||
| return _chains[coinType] if coinType in _chains else None | |||
| return _chains.get(parseCoinType(coinType), None) | |||
There was a problem hiding this comment.
default is None, but verbose is ok with me too.
decred/decred/wallet/simple.py
Outdated
| Copyright (c) 2019, Brian Stafford | ||
| Copyright (c) 2019, The Decred developers |
There was a problem hiding this comment.
As mentioned in my #53 review, this large pull requests are hard to review. Could we keep them smaller, ideally under 500 lines total?
One way could be to only change either decred or tinywallet code in each PR. If the change in the decred API would break tinywallet, and we want to keep master working at all times, one would include the bare minimum of tinywallet changes to keep it working, but reserve larger code changes to a dedicated tinywallet PR.
Also: much new code using os.path. I thought we agreed that pathlib.Path was the better alternative?
decred/decred/util/chains.py
Outdated
| @@ -93,5 +93,4 @@ def chain(coinType): | |||
| Returns: | |||
| obj: Network parameters. | |||
| """ | |||
| coinType = parseCoinType(coinType) | |||
| return _chains[coinType] if coinType in _chains else None | |||
| return _chains.get(parseCoinType(coinType), None) | |||
|
I switched to As far as PR size, you point is well taken, but it's not always going to work out quite so clean and neat, especially as we hack out basic features. I only mention it because there are likely some large and far-reaching PRs coming up that are certain to be over 500 loc diff. If you're not comfortable putting your stamp on something because it's too big, which is totally understandable, just give me a heads up. |
Step 1 of a few to implement full account functionality through the UI. Refactors a lot of UI code and simplifies the Wallet API. Also adds a SimpleWallet type, which is a single-account Decred wallet. Adds an account selection screen. Right now, there is still only one account available, and no option to create more. That will follow in the next step.
buck54321
force-pushed
the
accountsui
branch
2 times, most recently
from
a466343
to
ea68e2e
Compare
| if not oldThread.isFinished(): | ||
| if not oldThread.completed: |
There was a problem hiding this comment.
Relying on the finished signal here was problematic for recursively created threads with connection type Qt.DirectConnection. We are less interested in whether the QThread.run method has completed, and more interested in whether the callback function has been called.
Step 1 of a few to implement full account functionality through the UI. Refactors a lot of UI code and simplifies the Wallet API. Also adds a SimpleWallet type, which is a single-account Decred wallet.
Adds an account selection screen. Right now, there is still only one account available, and no option to create more. That will follow in the next step.