Skip to content
This repository has been archived by the owner on May 10, 2018. It is now read-only.

Add :stack to Job::Queue #365

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Add :stack to Job::Queue #365

wants to merge 6 commits into from

Conversation

svenfuchs
Copy link
Contributor

I have also refactored Job::Queue a little bit to remove all the repetition in matches_something? that we've had.

@queues ||= Array(Travis.config.queues).compact.map do |queue|
Queue.new(*queue.values_at(*[:queue, :slug, :owner, :language, :os]))
end
queues.detect { |queue| queue.send(:matches?, config) } || default
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just queue.matches?(config)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For some reason all the instance methods are private, not sure why

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@henrikhodne made it public

@queues ||= Array(Travis.config.queues).compact.map do |queue|
Queue.new(*queue.values_at(*[:queue, :slug, :owner, :language, :os]))
end
queues.detect { |queue| queue.matches?(config) } || default
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is fine as it is, but here's a Ruby tip of the day: Enumerable#detect takes an argument that's used if nothing matches, so this could be written as:

queues.detect(default) { |queue| queue.matches?(config) }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nice one, will use that

@svenfuchs
Copy link
Contributor Author

Can we merge this? :)

@svenfuchs
Copy link
Contributor Author

Added a feature flag for this (per owner) and tested on staging.

@joshk
Copy link
Contributor

joshk commented Mar 19, 2014

Can you explain how :stack is supposed to be used? Is it specified in the .travis.yml or in the hub config? Why would :stack be used used vs the other routing means?

@svenfuchs
Copy link
Contributor Author

@joshk during beta we whitelist repo owners using a feature flag. people can then opt in to using docker by setting stack: docker in their .travis.yml

All of language, os, stack currently behave exactly the same: if a queue defined in the gatekeeper config matches the attribute given in .travis.yml then the queue name from the gatekeeper config is used for the build.

The reason to pick stack over os was that potentially we might offer other OS's to run in Docker. Docker just isn't an OS ... and we couldn't come up with a better name for the flag. virtualization was discussed, but seemed too clunky.

@joshk joshk mentioned this pull request Jun 26, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants