-
Notifications
You must be signed in to change notification settings - Fork 437
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
fixed NS #194
fixed NS #194
Conversation
It definitely works at my side. Now I wonder how? |
use Interop\Amqp\AmqpDestination as InteropAmqpDestination; | ||
use Interop\Amqp\AmqpMessage as InteropAmqpMessage; | ||
use Interop\Amqp\AmqpQueue as InteropAmqpQueue; | ||
use Interop\Amqp\AmqpTopic as InteropAmqpTopic; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we get rid of these imports and use Enqueue\AmqpExt\AmqpConsumer
instead.
Same goes to other imported interfaces
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could, but the same I did above has been done in other classes.
I suggest to keep it that way for now and refactor mid-term. What do you think?
I can reproduce on PHP 5.6.31 by running examples/consume.php. |
@@ -2,11 +2,11 @@ | |||
|
|||
namespace Enqueue\AmqpExt; | |||
|
|||
use Interop\Amqp\AmqpConsumer; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be enought to remove this single line use Interop\Amqp\AmqpConsumer;
. It must work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since AmqpConsumer from Enqueue implements InteropAmqpConsumer, it should work indeed! I'm gonna give it a quick test and keep you posted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works! Confirmed by testing on 5.6.31. Should work on other versions as well. let's go for it :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chdeliens great! Could you please update the PR?
was causing "PHP Fatal error: Cannot use Interop\Amqp\AmqpConsumer as AmqpConsumer because the name is already in use in enqueue/amqp-ext/Flags.php on line 5"