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

Documentation, reorganization and additon of new methods #142

Merged
merged 27 commits into from
Feb 10, 2021

Conversation

germartinez
Copy link
Member

@germartinez germartinez commented Feb 8, 2021

Check #133, #89 and #139

@germartinez germartinez changed the title Reorganization and additon of new methods Documentation, reorganization and additon of new methods Feb 8, 2021
src/CPK.ts Outdated
*/
get address(): Promise<Address | undefined> {
if (this.#safeAppsSdkConnector?.isSafeApp) {
return (async () => this.#safeAppsSdkConnector && (await this.#safeAppsSdkConnector.getSafeInfo()).safeAddress)()
Copy link
Member

Choose a reason for hiding this comment

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

I'm definitely lacking knowledge on the architecture of cpk and stuff, but is it possible to cache the address when connecting?

Comment on lines +14 to +15
this.#appsSdk.getSafeInfo().then((appInfo: SafeInfo) => {
this.#isSafeApp = !!appInfo.safeAddress
Copy link
Member

Choose a reason for hiding this comment

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

here you go, I think you can just save the address and remove the async getter

@@ -0,0 +1,19 @@
type Networks = {
[key: string]: number
Copy link
Member

Choose a reason for hiding this comment

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

to be more precise I think we can also define a type for network names

@germartinez germartinez merged commit b95a05a into development Feb 10, 2021
@germartinez germartinez deleted the new-methods branch February 10, 2021 16:44
@germartinez germartinez mentioned this pull request Feb 22, 2021
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.

2 participants