You are viewing a single comment's thread from:

RE: Adjust Account Authorities with Steem Keychain

in #utopian-io5 years ago

Thank you for your contribution!

  1. I find very little comments in your code, a little bit more would be nice for others to understand.
  2. In general, you don't want to put too much code in the case - which bloats the entire switch statement - hard to maintain. You might want to extract the logics in a separate method - and unit test them.
  3. You might want to add messages to hint the users what is going on, instead of throwing errors in the console see below.

Your contribution has been evaluated according to Utopian policies and guidelines, as well as a predefined set of questions pertaining to the category.

To view those questions and the relevant answers related to your post, click here.


Need help? Chat with us on Discord.

[utopian-moderator]

Sort:  

Thanks for the feedback!

Yes, generally agreed on code structure. It's not really an option except as a refactor, which would need some coordination with the repository owner, but definitely something to consider for maintenance.

Also the console errors I presume are from the test page. In general we defer proper error handling to the clients, we have proper error codes set up, but this one is a precursor to that because this error stems from the steem keychain not being available at all (responsibility of client to check availability).

Thank you for your review, @justyy! Keep up the good work!

Curious.. how is it cool that the review comment here earns about $13 from @utopian-io but the actual post itself gets nothing? The comment clearly wasn't reviewing something that wasn't worth the effort of a review.

Explain?

Coin Marketplace

STEEM 0.27
TRX 0.13
JST 0.032
BTC 64693.66
ETH 2975.51
USDT 1.00
SBD 3.70