Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor send asset function code #405

Merged
merged 4 commits into from
Dec 10, 2017

Conversation

dvdschwrtz-zz
Copy link
Collaborator

What current issue(s) from Trello/Github does this address?

What problem does this PR solve?

How did you solve this problem?

How did you make sure your solution works?

Are there any special changes in the code that we should be aware of?

Is there anything else we should know?

  • Unit tests written?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) to 40.159% when pulling ed76b85 on dvdschwrtz:refactor-sendassetfn into 19409a5 on CityOfZion:dev.

@@ -67,25 +67,19 @@ export const sendTransaction = (sendAddress: string, sendAmount: string, symbol:

const isHardwareSend = !!publicKey
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a better check for this?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can define the type of the login? Feels like this condition is only known to us..

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, that makes sense. I will incorporate those changes for the ledger login page as well

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.0003%) to 40.079% when pulling ca79c3c on dvdschwrtz:refactor-sendassetfn into 430eaf9 on CityOfZion:dev.

@evgenyboxer evgenyboxer merged commit 5abc34c into CityOfZion:dev Dec 10, 2017
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.

None yet

4 participants