Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable wait()ing for a fractional number of seconds #80

Closed
wants to merge 1 commit into from

Conversation

@gjedeer
Copy link

commented May 22, 2013

This patch makes it possible to pass a fractional number of seconds to AMQPChannel::wait(), which is useful if you just want to check if there's a new result and quickly return.

This patch makes it possible to pass a fractional number of seconds to AMQPChannel::wait(), which is useful if you just want to check if there's a new result and quickly return.
@gjedeer

This comment has been minimized.

Copy link
Author

commented Jan 7, 2014

Any chance to pull this in? It's a trivial change and amqplib support in celery-php kind of depends on it.

@videlalvaro

This comment has been minimized.

Copy link
Collaborator

commented Jan 7, 2014

Sorry for not taking care of this sooner.

Question. The math method you are using, isn't it equivalent to this: https://github.com/videlalvaro/php-amqplib/blob/master/PhpAmqpLib/Wire/IO/StreamIO.php#L30 ?

If yes, on which version should be standardise?

@gjedeer

This comment has been minimized.

Copy link
Author

commented Jan 8, 2014

Thank you for taking care about that!

They seem to do the same thing, except mine multiplies the fractional part by 1000000 to get nanoseconds.

The one you quoted is more readable. I'd say, go for this one.

@ricardclau

This comment has been minimized.

Copy link

commented on dc42135 Jan 14, 2014

It should be $this->timeout in the second argument but 👍 to this change...

Not sure if I prefer that approach to separate integer and microseconds or the one I took at php-amqplib@05ef330

Thoughts? @gjedeer @videlalvaro

This comment has been minimized.

Copy link

replied Jan 14, 2014

Let's standardise on the one by @ricardclau

If someone could fix the timeout on the second arg, then I can merge the change

This comment has been minimized.

Copy link
Owner Author

replied Jan 14, 2014

How about this: a5dd97e

@ricardclau

This comment has been minimized.

Copy link
Contributor

commented Jan 14, 2014

@gjedeer I added it at #143 with a common function and a test for it

@gjedeer

This comment has been minimized.

Copy link
Author

commented Jan 23, 2014

Thank you!

@gjedeer gjedeer closed this Jan 23, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.