remove RSN4c, add RTS4b (channels#release)#1057
Conversation
`channels#release` in should raise error in realtime for specific states and should return without error in rest
tiholic
requested review from
QuintinWillison and
SimonWoolf
and removed request for
QuintinWillison
tiholic
changed the title
| *** @(RTS3c1)@ If a new set of @ChannelOptions@ is supplied to @Channels#get@ that would trigger a reattachment of the channel if supplied to @Channel#setOptions@ per "@RTL16a@":#RTL16a, it must raise an error, informing the user that they must use @Channel#setOptions@ instead | ||
| * @(RTS4)@ @Channels#release@ function: | ||
| ** @(RTS4a)@ Detaches the channel and then releases the channel resource i.e. it's deleted and can then be garbage collected | ||
| ** @(RTS4b)@ Calling @release()@ with a channel name that corresponds to a channel that is in any state other than @INITIALIZED@, @DETACHED@, or @FAILED@ must, instead of releasing the channel, raise an error with code @90001@ |
There was a problem hiding this comment.
So now these two spec items contradict each other - if you call release on an attached channel, should it detach then release, or raise an error?
My suggestion in the slack thread was just get rid of RSN4c entirely
There was a problem hiding this comment.
@SimonWoolf I lost context after a long gap. Removing RTS4b is what we are looking for, is it?
There was a problem hiding this comment.
Resolving this conversation on @SimonWoolf's behalf.
|
@tiholic @SimonWoolf @QuintinWillison - is there any follow up on this? Are we able to resolve? If not should I close this PR? Thanks! |
|
@tbedford we can land this PR after make necessary changes. Thanks for your patience :) |
|
This pull request will need more work. See this internal discussion in relation to the safe removal of spec points, where those spec points are at the end of a list. |
|
@QuintinWillison @tbedford @SimonWoolf necessary changes have been made. |
|
@tbedford This is ready to land but I'll leave that to your team, when you're ready. |
|
@QuintinWillison - Looks good to me, however I can't merge until the feedback from @SimonWoolf is attended to. Thanks. |
see discussion: #1057 (comment)
see discussion: ably/docs#1057 (comment)
see discussion: ably/docs#1057 (comment)
see discussion: ably/docs#1057 (comment)
6 participants
channels#releasein should raise error inrealtimefor specific states and should return without error inrestRSN4c should be put under RTS4
Discussion: https://ably-real-time.slack.com/archives/C030C5YLY/p1615715227000800