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

Make rbenv_ruby optional #67

Merged
merged 1 commit into from
Dec 4, 2016

Conversation

spikeheap
Copy link
Contributor

This PR addresses #61 and resolves the issue where the Ruby version changes between the current and new releases.

By removing the requirement for :rbenv_ruby, we can delegate the decision to the remote host's rbenv, which will follow these steps to choose a version. This mitigates the above problem by using the 'old' Ruby version to run commands on current, and the 'new' Ruby version on the release in progress.

@mattbrictson
Copy link
Member

This seems reasonable to me, but I don't use capistrano-rbenv and therefore don't feel comfortable merging without further review. Can anyone else provide feedback on this PR?

@spikeheap
Copy link
Contributor Author

It would be great to see this merged. Is there anything I could do to increase it's chances of getting merged?

@mattbrictson
Copy link
Member

@kirs @leehambley Any strong opinions on this one?

@doits
Copy link

doits commented Nov 22, 2016

+1 for me - I'd like the app server to decide which ruby version to use, and when the app server upgrades its ruby version, why do I have to update every deploy config of every single app? So having this optional - forcing a ruby version if I really want/need to, otherwise simply using the global ruby version - seems to be a good way to make everybody happy.

@mattbrictson
Copy link
Member

OK, I'll merge this in. Thanks!

@mattbrictson mattbrictson merged commit 332f204 into capistrano:master Dec 4, 2016
@spikeheap spikeheap deleted the optional_rbenv_ruby branch December 4, 2016 20:08
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