chat: mention newestFirst in history spec #250
Conversation
textile/chat-features.textile
Outdated
| @@ -299,7 +299,7 @@ Broadly speaking, messages are published via REST calls to the Chat HTTP API and | |||
| ** @(CHA-M5c)@ @[Testable]@ If a channel leaves the @ATTACHED@ state and then re-enters @ATTACHED@ with @resumed=false@, then it must be assumed that messages have been missed. The @subscription point@ of any subscribers must be reset to the @attachSerial@. | |||
| ** @(CHA-M5d)@ @[Testable]@ If a channel @UPDATE@ event is received and @resumed=false@, then it must be assumed that messages have been missed. The @subscription point@ of any subscribers must be reset to the @attachSerial@. | |||
| ** @(CHA-M5e)@ Each subscription shall expose a method or callback that allows for messages to be queried. These messages are queried via the "REST API"#rest-fetching-messages. | |||
| ** @(CHA-M5f)@ @[Testable]@ This method must accept any of the standard history query options, except for @direction@, which must always be @backwards@. | |||
| ** @(CHA-M5f)@ @[Testable]@ This method must accept any of the standard history query options, except for @direction@, which must always be @newestFirst@. | |||
AndyTWF
force-pushed
the
chat-history-enums
branch
from
2dae4de to
d28ca19
Compare
| @@ -299,7 +299,7 @@ Broadly speaking, messages are published via REST calls to the Chat HTTP API and | |||
| ** @(CHA-M5c)@ @[Testable]@ If a channel leaves the @ATTACHED@ state and then re-enters @ATTACHED@ with @resumed=false@, then it must be assumed that messages have been missed. The @subscription point@ of any subscribers must be reset to the @attachSerial@. | |||
| ** @(CHA-M5d)@ @[Testable]@ If a channel @UPDATE@ event is received and @resumed=false@, then it must be assumed that messages have been missed. The @subscription point@ of any subscribers must be reset to the @attachSerial@. | |||
| ** @(CHA-M5e)@ Each subscription shall expose a method or callback that allows for messages to be queried. These messages are queried via the "REST API"#rest-fetching-messages. | |||
| ** @(CHA-M5f)@ @[Testable]@ This method must accept any of the standard history query options, except for @direction@, which must always be @backwards@. | |||
| ** @(CHA-M5f)@ @[Testable]@ This method must accept any of the standard history query options, except for @orderBy@, which must always be @newestFirst@. | |||
There was a problem hiding this comment.
| ** @(CHA-M5f)@ @[Testable]@ This method must accept any of the standard history query options, except for @orderBy@, which must always be @newestFirst@. | |
| ** @(CHA-M5f)@ @[Testable]@ This method can accept any of the standard history query options, except for @orderBy@, which must always be @newestFirst@. |
There was a problem hiding this comment.
The original wording is correct here imo - it's saying the method must accept any of the standard arguments, i.e. its talking about the method signature. It would not be acceptable for the method not to accept the limit param, for example.
There was a problem hiding this comment.
But, we also have default values for QueryParam arguments.
Okay maybe
| ** @(CHA-M5f)@ @[Testable]@ This method must accept any of the standard history query options, except for @orderBy@, which must always be @newestFirst@. | |
| ** @(CHA-M5f)@ @[Testable]@ This method signature must have all history query options as params, except for @orderBy@, which must always be @newestFirst@. |
This feels less confusing. Because there's a diff. in accepting the argument and defining it as a part of method signature.
There was a problem hiding this comment.
This feels more explicit and there won't be implicit misunderstandings wdyt
There was a problem hiding this comment.
Downside with this wording is that it would be overly-prescriptive. This wording strongly requires that each history query option must be an individual arg to the function - that's not the case in JS (the history options are an object - which is common in other languages for the core SDK too).
There was a problem hiding this comment.
Okay, got it.
But, I would still recommend to make statement more explicit that fits across SDKs in general.
I will approve the PR for now 👍
No description provided.