Solve weekly technical debt #13
Conversation
dmytrostriletskyi
requested review from
Art17,
Alladin9393 and
anastasiia-bilova
Codecov Report
@@ Coverage Diff @@
## develop #13 +/- ##
===========================================
+ Coverage 78.78% 98.98% +20.2%
===========================================
Files 7 9 +2
Lines 66 99 +33
===========================================
+ Hits 52 98 +46
+ Misses 14 1 -13
Continue to review full report at Codecov.
|
dmytrostriletskyi
force-pushed
the
weekly-tech-debt
branch
2 times, most recently
from
fd42bc7
to
0f01dc7
Compare
dmytrostriletskyi
force-pushed
the
weekly-tech-debt
branch
from
0f01dc7
to
52616d6
Compare
README.md
Outdated
| @@ -94,7 +102,7 @@ Get balance of the account by its address — ``remme account get-balance``: | |||
|
|
|||
| | Arguments | Type | Required | Description | | |||
| | :-------: | :----: | :-------: | --------------------------------------------------- | | |||
| | address | String | Yes | Get balance of the account by its address. | | |||
| | address | String | Yes | Account address to get a balance by. | | |||
| | node-url | String | No | Apply the command to the specified node by its URL. | | |||
There was a problem hiding this comment.
Node URL to apply the command to.
There was a problem hiding this comment.
@Art17, done.
README.md
Outdated
|
|
||
|  | ||
| 1. [Branch isn't out-of-date with the base branch](https://habrastorage.org/webt/ux/gi/wm/uxgiwmnft08fubvjfd6d-8pw2wq.png). | ||
| 2. [Have wrote the description of the pull request and have added at least 2 reviewers](https://github.com/camo/55c309334a8b61a4848a6ef25f9b0fb3751ae5e9/68747470733a2f2f686162726173746f726167652e6f72672f776562742f74312f70792f63752f7431707963753162786a736c796f6a6c707935306d7862357969652e706e67). |
There was a problem hiding this comment.
Have wrote > Have written.
There was a problem hiding this comment.
@Art17, done.
tests/test_account.py
Outdated
| ]) | ||
|
|
||
| assert PASSED_EXIT_FROM_COMMAND_CODE == result.exit_code | ||
| assert True is isinstance(json.loads(result.output), int) |
There was a problem hiding this comment.
assert isinstance(json.loads(result.output), int)
There was a problem hiding this comment.
@Art17, done.
|
|
||
| remme = Remme(private_key_hex=None, network_config={ | ||
| remme = Remme(network_config={ |
There was a problem hiding this comment.
Create a function that returns Remme object, because its code reuses all functions in command/cli.py.
Like here: click
There was a problem hiding this comment.
@Alladin9393, there is no:
if node_url is None:
node_url = 'localhost'
anymore. The initialization of the remme object only. I do not think, we should create a separated function for object creation.
There was a problem hiding this comment.
@Alladin9393, maybe we could write the code in the following way:
remme = Remme(network_config={'node_address': str(node_url) + ':8080', })
To make it smaller. What do you think?
There was a problem hiding this comment.
@dmytrostriletskyi, Ok, let it be as it is now.
cli/account/cli.py
Outdated
| }) | ||
|
|
||
| if errors: | ||
| click.echo(dict_to_pretty_json(errors)) |
There was a problem hiding this comment.
Stylize please error output.
Example
There was a problem hiding this comment.
@Alladin9393, your approach has:
- Additional function.
- Which is based on
marshmallow(so it has dependency).
We could remove additional function, and just do the following:
pprint(dict_to_pretty_json(errors))And the result would be the same but without library's dependency using already created custom code in dict_to_pretty_json.
There was a problem hiding this comment.
@dmytrostriletskyi, ok, I think its better pprint(dict_to_pretty_json(errors)).
dmytrostriletskyi
force-pushed
the
weekly-tech-debt
branch
from
9c2857a
to
01dab3e
Compare
dmytrostriletskyi
force-pushed
the
weekly-tech-debt
branch
from
01dab3e
to
accbc1a
Compare
Solve weekly technical debt:
f-notation,FAILED_EXIT_FROM_COMMAND>FAILED_EXIT_FROM_COMMAND_CODE,