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

Update README.md #58

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Update README.md #58

wants to merge 1 commit into from

Conversation

hbin
Copy link

@hbin hbin commented Nov 20, 2015

This line is misleading. It will override the rbenv_map_bins, and will lead to some unexpected issue to others packages.

seuros/capistrano-sidekiq#124

This line is misleading. It will override the `rbenv_map_bins`, and will lead to some unexpected issue to others packages.

seuros/capistrano-sidekiq#124
@amw
Copy link

amw commented Dec 20, 2015

This is how you actually use this gem. Without that line these commands would not be executed through rbenv.

@hbin
Copy link
Author

hbin commented Dec 21, 2015

@amw These was set by default

@amw
Copy link

amw commented Dec 21, 2015

Yes, I know, but do you mean that if you remove that line from your deploy config it will help with compatibility with other plugins? It would be a design problem of these other plugins then.

It is an option that rbenv exposes and users are allowed to add additional commands to that list. This line in README just explicitly sets the var to it's default value which is a nice and standard way to present the options to users.

ryush00 referenced this pull request in seuros/capistrano-puma May 17, 2016
require updated version of puma/capistrano
add plugins support
@will-in-wi
Copy link
Contributor

This is certainly an issue, although I don't think removing it from the readme is the right solution…

The deeper issue is that we are recommending that the end user set this when really, they shouldn't unless they need to.

Here's a different approach: #76

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