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

Queue.subscribe! #20

Merged
merged 7 commits into from
Jun 14, 2016
Merged

Queue.subscribe! #20

merged 7 commits into from
Jun 14, 2016

Conversation

baelter
Copy link
Contributor

@baelter baelter commented Jun 11, 2016

@coveralls
Copy link

coveralls commented Jun 11, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 5fc52d1 on baelter:master into 1f045d3 on arempe93:master.

@arempe93
Copy link
Owner

Thanks for this implementation! I will look at it more closely later tonight

@coveralls
Copy link

coveralls commented Jun 12, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 6d99da0 on baelter:master into 1f045d3 on arempe93:master.

@baelter
Copy link
Contributor Author

baelter commented Jun 12, 2016

You can hold this PR until further notice, I've found some shortcomings I want to fix.

@coveralls
Copy link

coveralls commented Jun 13, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling ba08f62 on baelter:master into 1f045d3 on arempe93:master.

@baelter
Copy link
Contributor Author

baelter commented Jun 13, 2016

Ok @arempe93, happy with this now.

Added support for multiple routes with them same key. Basically Exchange::routes hash items are arrays of routes instead.

@arempe93
Copy link
Owner

Looks pretty good to me - only question is why call yield_consumers in the subscribe method? I can't think of a case where it would do anything - but maybe there is

@baelter
Copy link
Contributor Author

baelter commented Jun 14, 2016

Reason for that is that if there are messages in the queue they will be delivered when a consumer subscribes.

@arempe93
Copy link
Owner

Ok - that makes sense. For some reason I didn't think of the case that there could be messages entered into the queue before a subscription happens.

@arempe93 arempe93 merged commit 09091e4 into arempe93:master Jun 14, 2016
@baelter
Copy link
Contributor Author

baelter commented Jun 14, 2016

Sweet, when are u planning next release?

@arempe93
Copy link
Owner

Probably when I have time after work today. I don't think there's anything else in progress right now

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.

3 participants