Skip to content

Conversation

@krisbitney
Copy link
Contributor

No description provided.

@krisbitney krisbitney requested a review from dOrgJelli as a code owner August 9, 2023 19:15
@dOrgJelli
Copy link
Contributor

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

@krisbitney
Copy link
Contributor Author

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

Comment on lines +191 to +199
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
Copy link
Contributor

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

Copy link
Contributor Author

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

Copy link
Contributor Author

@krisbitney krisbitney Aug 31, 2023

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.

Copy link
Contributor

@dOrgJelli dOrgJelli left a 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.

@dOrgJelli dOrgJelli merged commit a0f57fe into wrap-0.1 Aug 30, 2023
@krisbitney krisbitney deleted the kris/kotlin-app-bindings branch August 31, 2023 15:25
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.

4 participants