-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add support for MoveFunds action #22
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good
toPot, | ||
} = event.args; | ||
|
||
const colonyClient = await networkClient.getColonyClient(colonyAddress); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we could optimize the ingestor somewhat if we were to store all known instances of the colony client.
I see us re-instantiating the client for a lot of tracked events.
But I actually don't know what this impact would be real-life.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍👍
const isSupportedColonyClient = ( | ||
colonyClient: AnyColonyClient, | ||
): colonyClient is SupportedColonyClient => | ||
(colonyClient as SupportedColonyClient).getDomainFromFundingPot !== undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
alternatively:
'getDomainFromFundingPot' in colonyClient
Then you don't need to cast colonyClient
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think explicit casting might be better here since it will alert you if the property you're checking against doesn't exist/got removed
https://www.typescriptlang.org/docs/handbook/2/narrowing.html#using-type-predicates
This is a parallel PR to JoinColony/colonyCDapp#291
Please refer to that PR for testing instructions