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

Allow GrpcClient to be implemented on all platforms #2677

Merged
merged 1 commit into from
Oct 18, 2023

Conversation

oldergod
Copy link
Member

Including JVM or common. Following up on #2505 and we didn't quite do that.
Obviously breaking some APIs. Most common breaking change for client users is around having now to import newCall or create for they became extension functions.
I am not really sure it's the right way.

Goals are to let anybody implement their own GrpcClient on common, jvm, js, native. Right now, that's only possible for js and native. With this PR, I think all would be cool.

Fixes #2484
cc @iTaysonLab @RaphaelJenni

@oldergod oldergod force-pushed the bquenaudon.2023-10-12.grpcclientopenforreal branch from 46c314a to 5cf9587 Compare October 12, 2023 14:50
@oldergod
Copy link
Member Author

Jesse is telling me we could create the option at the OkHttp level. We'll pair next week and see how it goes.

Copy link
Member

@swankjesse swankjesse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great!

public synthetic fun <init> (Lokhttp3/Call$Factory;Lokhttp3/HttpUrl;JLkotlin/jvm/internal/DefaultConstructorMarker;)V
public final fun create (Lkotlin/reflect/KClass;)Lcom/squareup/wire/Service;
public final fun newBuilder ()Lcom/squareup/wire/GrpcClient$Builder;
public fun <init> ()V
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public fun <init> ()V
public fun <init> ()V
public final fun create (Lkotlin/reflect/KClass;)Lcom/squareup/wire/Service;
public final fun newBuilder ()Lcom/squareup/wire/GrpcClient$Builder;```

Comment on lines 42 to 46
public final class com/squareup/wire/GrpcClientKt {
public static final fun create (Lcom/squareup/wire/GrpcClient;Lkotlin/reflect/KClass;)Lcom/squareup/wire/Service;
public static final fun newBuilder (Lcom/squareup/wire/GrpcClient;)Lcom/squareup/wire/GrpcClient$Builder;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn’t be a public API.

Suggested change
public final class com/squareup/wire/GrpcClientKt {
public static final fun create (Lcom/squareup/wire/GrpcClient;Lkotlin/reflect/KClass;)Lcom/squareup/wire/Service;
public static final fun newBuilder (Lcom/squareup/wire/GrpcClient;)Lcom/squareup/wire/GrpcClient$Builder;
}

@oldergod oldergod force-pushed the bquenaudon.2023-10-12.grpcclientopenforreal branch from 5cf9587 to 5ee8d73 Compare October 18, 2023 11:18
@oldergod oldergod force-pushed the bquenaudon.2023-10-12.grpcclientopenforreal branch from 5ee8d73 to 82e565a Compare October 18, 2023 11:21
@oldergod
Copy link
Member Author

Updated the code!

@oldergod oldergod merged commit 0767120 into master Oct 18, 2023
13 of 14 checks passed
@oldergod oldergod deleted the bquenaudon.2023-10-12.grpcclientopenforreal branch October 18, 2023 16:13
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.

Ability to use custom gRPC clients on Multiplatform
2 participants