-
Notifications
You must be signed in to change notification settings - Fork 52
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
Remove all mentioning of rbenv from phpenv output #30
base: master
Are you sure you want to change the base?
Conversation
This seems incomplete, there are more matches for ruby, like .ruby-version or RUBY_PATH="$(phpenv-which ruby)", so perhaps add such replace:
|
|
||
phpenv_script() { | ||
local root="$1" | ||
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove trailing whitespace.
… docu to include both phpenv and rbenv, some comment/minor text changes
95e7852
to
55192f1
Compare
@glensc |
@DennisBirkholz i've managed to do full replacement, and it seems to work excellent: https://github.com/pld-linux/phpenv/blob/auto/th/phpenv-0.4.0-1/phpenv.spec#L38-L73 i've done some comparing to chh/phpenv, such solution supports split trees: code in |
@glensc I agree with you that a version which completely replaces all ruby/rbenv and supports split trees is preferable. Your .spec-based solution however is limited to RPM based systems. I don't know what the perfect solution for phpenv is however and I don't have the time ATM. Maybe the best way is to generalize rbenv to some scriptenv or langenv that is language agnostic itself and handles languages via configuration and not forking... |
@DennisBirkholz it's not rpm based solutuion. i just inlined the shell script there. if you look closer it's just shell script there. |
Hi Christoph,
my patch replaces all mentioning of rbenv from phpenv (but keeps a reference to rbenv in the help text). Please consider including the patch, thx.
Greets,
Dennis