Skip to content
This repository was archived by the owner on Feb 23, 2021. It is now read-only.

Conversation

@tanx
Copy link
Contributor

@tanx tanx commented Aug 20, 2018

Closes #510

@tanx tanx requested a review from valentinewallace August 20, 2018 09:54
@tanx tanx mentioned this pull request Aug 20, 2018

class ChannelAction {
constructor(store, grpc, nav, notification) {
constructor(store, grpc, transaction, nav, notification) {
Copy link

Choose a reason for hiding this comment

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

Out of curiosity - what's the logic behind adding a new param in the middle of existing ones rather than to the end? Also, in case of 2+ args I'm a proponent of passing in an object so that the ordering does not matter.

Copy link
Contributor

@valentinewallace valentinewallace left a comment

Choose a reason for hiding this comment

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

LGTM 👍 seems like a clean/simple fix pending wallet balance streaming.

@valentinewallace valentinewallace merged commit 6ae9e66 into master Aug 21, 2018
@valentinewallace valentinewallace deleted the update-wallet-on-channel-event branch August 21, 2018 00:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants