airgap-it / beacon-android-sdk

The beacon sdk allows Android developers of dApps and wallets on Tezos to implement the wallet interaction standard tzip-10.
https://walletbeacon.io
MIT License
10 stars 8 forks source link

SDK improvement suggestions #4

Open AndreasGassmann opened 2 years ago

AndreasGassmann commented 2 years ago

In our last sync call with other teams, a couple improvements were suggested. They are relevant to both iOS and Android, some also concern the protocol overall, so I will add everything here.

Overall

Android

iOS

jsamol commented 2 years ago

Android

In an earlier version, it was possible to construct a response from a request. It seems that this is no longer possible. Would it be possible to add the functionality back?

It is still possible. For example, TransferSubstrateResponse can be created from TransferSubstrateRequest using one of the TransferSubstrateResponse.Companion's methods. Similar methods are also defined for other responses' companion objects.

The HTTP Interface should be exposed to the developer and the default implementation should be moved to a separate package. This would allow developers to have fewer dependencies.

That's a valid point. Similarly, the CryptoProvider default implementation could be extracted to a separate package as well.

iOS

The SDK is not released to cocoapods. It would make it easier to update to new versions if the SDK was on cocoapods. However, it was mentioned that it's possible to directly add the SDK from github because a specfile is present. If that's the case, then we should document how this is possible.

We're not planning on releasing to CocoaPods. But yes, it's possible to add the dependency directly from GitHub. I'll make sure the pod spec file is up to date and update README with detailed information on how to do it.

Currently, the part of the code that connects to Matrix is a singleton. It would be helpful if the user had more control over this object.

MatrixClient and its dependencies are not singletons. What does it mean to "have more control over this object"?

ERussel commented 2 years ago

iOS

We're not planning on releasing to CocoaPods. But yes, it's possible to add the dependency directly from GitHub. I'll make sure the pod spec file is up to date and update README with detailed information on how to do it.

I have tried current podspec file from here. There are several building issues due to the fact that Cocoapods builds single framework from subspecs:

Looks like we need a separate podspecs instead of subspecs.

jsamol commented 2 years ago

@ERussel I moved the CocoaPods discussion to a separate issue in Beacon iOS SDK (#8).