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

SDK improvement suggestions #4

Open
AndreasGassmann opened this issue Feb 4, 2022 · 3 comments
Open

SDK improvement suggestions #4

AndreasGassmann opened this issue Feb 4, 2022 · 3 comments
Assignees

Comments

@AndreasGassmann
Copy link
Member

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

  • The response does not contain a "signature", only a "payload" and/or a "hash". It is unclear where the signature is. Also, the "payload" might not always be necessary, so it would make sense to add an additional property for the signature.
  • Additional properties might be required in a "request". We will look into the SignPayloadJSON type from @polkadotjs.
  • We should use Semantic Versioning and keep the versions across all SDKs in sync (Android, iOS, Typescript)

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?
  • 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.
  • Version 3.0.0 is already released. We will have to make sure we respect SemVer and keep all packages in sync.

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.
  • 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.
@jsamol
Copy link
Member

jsamol commented Feb 7, 2022

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"?

@jsamol jsamol self-assigned this Feb 7, 2022
@ERussel
Copy link

ERussel commented Feb 17, 2022

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:

  • duplicate file AppMetadata.swift
  • not needed 'import BeaconCore' in subspecs (for example, BeaconBlockchainSubstrate)
  • various definition duplicates, for example, ExtendedDependencyRegistry

Looks like we need a separate podspecs instead of subspecs.

@jsamol
Copy link
Member

jsamol commented Mar 1, 2022

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

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

No branches or pull requests

3 participants