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

Customize unicorn command #32

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

Conversation

spectator
Copy link

Lets you customize unicorn command to something other than bundle exec unicorn, for example bin/unicorn.

Also includes minor updates such as using current_path.join instead of
File.join.

Lets you customize unicorn command to something other than `bundle exec
unicorn`, for example `bin/unicorn`.

Also includes minor updates such as using `current_path.join` instead of
`File.join`.
@mlineen
Copy link
Member

mlineen commented Jul 14, 2014

Humm, I just rejected PR #28 because changing the command changes the meaning of the arguments particularly for -E when using unicorn vs unicorn_rails. I do see the value of being able to specify ./bin/unicorn though and I'm guessing you use binstubs.

My major concern with this change is that it will likely break the SSHKit::CommandMap. See the tl;dr here: https://github.com/capistrano/capistrano.

Are the updates for current_path.join just a syntax preference or was there any other motivation?

According to https://github.com/capistrano/capistrano#tasks, commands
with spaces gets executed slightly differently. To preserve 'better'
behaviour, use splat operator for the first argument to `execute` and make
default contain array of command chunks.

It is perfectly fine to set `unicorn_command` to just 'bin/unicorn',
splat can handle that.
@spectator
Copy link
Author

Thanks for pointing this out, I wan't aware of this behaviour. Could you please take a look at the second commit I have just pushed? It should preserve original behavior and let users customize how they want to run unicorn at their own peril.

In re current_path.join vs File.join. I believe current_path is just an instance of Pathname which already has everything needed to create proper paths, so it is weird to see Pathname and File mixed up together where just Pathname is more than enough. Also, looking at Capistrano source it seems more conventional to use current_path.join.

@soulcutter
Copy link
Contributor

I am favorable towards this.

@masterkain
Copy link

Would like to use Rainbows! with this, +1

@christhekeele
Copy link

I would like this as well. Hacking around it in the mean time.

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.

5 participants