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

kotlin app bindings #53

Merged
merged 12 commits into from
Aug 30, 2023
Merged

kotlin app bindings #53

merged 12 commits into from
Aug 30, 2023

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.

3 participants