-
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
kotlin app bindings #53
Conversation
In order to show the e2e developer experience, can we please create a template project that uses these bindings? Similar to: https://github.com/polywrap/cli/blob/origin-dev/packages/templates/app/typescript/src/index.ts |
Template PR is here: polywrap/wrap-cli#1867 |
companion object { | ||
val uri: Uri = Uri("testimport.uri.eth") | ||
} | ||
|
||
protected val defaultClient: Invoker by lazy { | ||
polywrapClient { addDefaults() } | ||
} | ||
protected val defaultUri: Uri? = null | ||
protected val defaultEnv: TestImportEnv? = null |
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.
As a non Kotlin dev, I am just curious why we aren't using same pattern as other clients where we use constructor and abstract methods that can be extended
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.
In Kotlin, abstract members can only be in an abstract class. If a user wants to, they can extend the class and override the members defaultClient
, defaultUri
, and defaultEnv
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 gave these properties the protected
access modifier, which makes them visible to this class and also its children. That way they are essentially private, but users who extend the class can still see and override them.
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.
This LGTM, we can continue to make improvements to it post-merge via new versions on wrapscan.
No description provided.