fix(methodObject): remove result as required#240
Conversation
| <a name="method-externalDocs"></a>externalDocs | [External Documentation Object](#external-documentation-object) | Additional external documentation for this method. | ||
| <a name="method-params"></a>params | [[Content Descriptor](#content-descriptor-object) \| [Reference Object](#reference-object) \| [OneOf Object](#oneof-object)] | **REQUIRED**. A list of parameters that are applicable for this method. The list MUST NOT include duplicated parameters and therefore require [name](#content-descriptor-name) to be unique. The list can use the [Reference Object](#reference-object) to link to parameters that are defined by the [Content Descriptor Object](#content-descriptor-object). It may also nest the content descriptor or reference object inside of a [OneOf Object](#oneof-object). All optional params (content descriptor objects with "required": false) MUST be positioned after all required params in the list. | ||
| <a name="method-result"></a>result | [Content Descriptor](#content-descriptor-object) \| [Reference Object](#reference-object) \| [OneOf Object](#oneof-object) | **REQUIRED**. The description of the result returned by the method. It MUST be a Content Descriptor. | ||
| <a name="method-result"></a>result | [Content Descriptor](#content-descriptor-object) \| [Reference Object](#reference-object) \| [OneOf Object](#oneof-object) | The description of the result returned by the method. It MUST be a Content Descriptor. Not providing a result field indicates a notification. |
There was a problem hiding this comment.
This seems wrong to me, and I recommend we reject the PR.
Every method has a result, even if that result is void. In JSON-RPC the client may decide it doesn't care to receive the result by sending a notification (omitting the id in the request), but that's a client decision and nothing to do with the full protocol of the schema the client can expect if it did not send a notification.
Now, if Open-RPC allows a server to document that it only supports notifications for some subset of its methods (that it would reject a request with an ID for those methods) then it makes sense to not have to document a result. Is that the case though, that servers or the Open-RPC spec would support only receiving a particular RPC method request without an id such that a result will never be sent?
There was a problem hiding this comment.
@AArnott I wrote a comment linked to your suggestion(s). I think you're right and I fully agree with you.
I would suggest to keep result required but allow it to be false. This would indicate that
a server (...) only supports notifications
What to you think about it ?
There was a problem hiding this comment.
Not a fan of false since the question/field isn't asking for a boolean answer. null might make more sense here. If we want to support notification-only schemas, then omitting result may make sense -- more sense IMO than setting it to false. But specifying it with a null value makes it more explicit and therefore noticeable.
There was a problem hiding this comment.
as i mentioned here: #230 (comment)
We can already define null results.
Bit I do agree with the criticism that omitting it is implicit vs explicit
|
since there is no consensus here, let's continue discussion in this issue: #230 |
|
reopening this in discussion of #230 |
fixes #230