search

sourcegraph / jsonrpc2

Package jsonrpc2 provides a client and server implementation of JSON-RPC 2.0 (http://www.jsonrpc.org/specification)
MIT License
190 stars 62 forks source link

apply method on CallOption should be public #43

Closed samherrmann closed 3 years ago

samherrmann commented 3 years ago

The apply method on the CallOption interface is private, making it not possible for a library user to set their own CallOption.

My use case: I am integrating with a device that only supports request IDs as a string. While I see that this library can parse request IDs as a number or a string, I do not see an obvious way to configure the library to send request IDs as a string. Maybe there is a way and I just don't see it? Maybe that should be another issue? My plan B was to create a CallOption that sets the ID.IsString property to true, but then I ran into the issue where the Go compiler complains because the apply method is private. I think in the meantime I can use jsonrpc2.PickID, but that will also require me to manage the ID assignment.

keegancsmith commented 3 years ago

Rather than making apply public, can you contribute a call option which sets IsString?

I don't remember the motivation behind making apply private, but there are possible downsides to arbitrary mutations to a Request. @sqs do you remember?

Do you have other ideas of good things people could do in apply that would motivate it being public? For now the simple fix is to contribute a new call option.

sqs commented 3 years ago

@keegancsmith:

I don't remember the motivation behind making apply private, but there are possible downsides to arbitrary mutations to a Request. @sqs do you remember?

I made it unexported because there was no need at the time to make it exported (ie wanting to keep the API surface as small as possible). And because it meant we didn't need to figure out how to prevent or document the possible consequences of mutating the Request.

Agree with @keegancsmith's suggestions/questions above.