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

Allow dynamic adding queues #322

Merged
merged 11 commits into from
Mar 6, 2017
Merged

Allow dynamic adding queues #322

merged 11 commits into from
Mar 6, 2017

Conversation

phstc
Copy link
Collaborator

@phstc phstc commented Feb 28, 2017

Fix #282

@phstc phstc force-pushed the allow-dynamic-adding-queues-282 branch from 49f5175 to f90521c Compare February 28, 2017 13:17
@@ -99,7 +99,7 @@ def prefix_active_job_queue_names
end

def parse_queue(queue, weight = nil)
[weight.to_i, 1].max.times { Shoryuken.queues << queue }
Shoryuken.add_queue(queue, [weight.to_i, 1])
Copy link
Contributor

Choose a reason for hiding this comment

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

You have not take the max of [weight.to_i, 1] but is it OK?
I think that add_queue takes weight which is defined numeric.

Copy link
Collaborator Author

@phstc phstc Mar 3, 2017

Choose a reason for hiding this comment

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

@h3poteto nice! thanks for reviewing it. I meant [weight.to_i, 1].max.

This code needs a test rspec. Not sure how tests are passing, even the integration one.

@@ -99,7 +99,7 @@ def prefix_active_job_queue_names
end

def parse_queue(queue, weight = nil)
Shoryuken.add_queue(queue, [weight.to_i, 1])
Shoryuken.add_queue(queue, [weight.to_i, 1].max)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@h3poteto 🍻

@phstc phstc merged commit 765e0de into v3 Mar 6, 2017
@phstc phstc deleted the allow-dynamic-adding-queues-282 branch March 6, 2017 03:36
@phstc phstc mentioned this pull request Mar 6, 2017
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