search

friends-of-reactphp / stomp

STOMP bindings for ReactPHP.
MIT License
115 stars 42 forks source link

Add `React\Stomp\Client::ack` and `React\Stomp\Client::nack` methods #2

Closed romainneutron closed 11 years ago

romainneutron commented 11 years ago

see http://stomp.github.com//stomp-specification-1.1.html#ACK see http://stomp.github.com//stomp-specification-1.1.html#NACK

romainneutron commented 11 years ago

My bad, I just realized the spec have changed between 1.1 and 1.2, I implemented for 1.2 :(

see http://stomp.github.com//stomp-specification-1.1.html#ACK see http://stomp.github.com//stomp-specification-1.2.html#ACK

1.1 requires 'message-id' header. I'm gonna revert this

romainneutron commented 11 years ago

PR updated, now 1.1 compatible (as of this client version requirement)

igorw commented 11 years ago

Those comments were intentional by the way, they are the API documentation :D

igorw commented 11 years ago

I discussed this with @jsor and using DeferredResolver is maybe not the best approach. It is an internal promise API, so perhaps we should make our own instead.

Here's a prototype of how it could work: https://gist.github.com/4081306

Using promises still makes sense for consuming acks though. So we should still use them for that.