Skip to content

#22 Add events on Consumer and RpcServer#23

Closed
thomasvargiu wants to merge 4 commits intoprolic:masterfrom
thomasvargiu:HumusAmqpModule-22
Closed

#22 Add events on Consumer and RpcServer#23
thomasvargiu wants to merge 4 commits intoprolic:masterfrom
thomasvargiu:HumusAmqpModule-22

Conversation

@thomasvargiu
Copy link
Copy Markdown
Contributor

Implemented events in consumer class.

Logger is not a consumer dependency anymore.
Callbacks and logger are not required, but compatible with the old configuration.
The result for deliveryCallback is the return value of the last listener. It's possible to set the right priorities or to stop execution in the listener.

I've done some little refactoring and tests.

@thomasvargiu
Copy link
Copy Markdown
Contributor Author

What do you think?

@prolic
Copy link
Copy Markdown
Owner

prolic commented Nov 3, 2014

Thank you very much for your contribution. The event system would also help to implement automatic serialization and encoding of the message (see #16).
Some small stuff should be done before merging this, please give me a little time to check this.

Comment thread composer.json Outdated
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The supervisor module should not be a hard dependency on the module, as this module works only on unix systems. So this would mean you cannot use the amqp module on windows platforms anymore.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just an error, we need it in require-dev. I'll move it.

@prolic
Copy link
Copy Markdown
Owner

prolic commented Nov 10, 2014

Looks good, I need to do some tests first, merge will happen the next days. Thank you!

prolic added a commit that referenced this pull request Nov 10, 2014
@prolic prolic closed this Nov 10, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants