-
Notifications
You must be signed in to change notification settings - Fork 110
Symlink native composer home #32
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
Symlink native composer home #32
Conversation
looks good. A few notes on the PATH variable:
|
I agree. I just changed according to previous implementation, let's listen what @fquffio thinks. |
To be honest I don't know if |
I didn't forget about this PR, but I hadn't time to take a look into it yet. I promise I'll do as soon as I can. @Slamdunk |
Free and open-source software maintainers aren't requested to respect any schedule. Take all the time you need 😉 |
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.
Seems ok. Just two minor concerns… if you don't have time to take care of those no problem.
5.4/Dockerfile
Outdated
RUN curl -sS https://getcomposer.org/installer | php -- --install-dir=/usr/local/bin --filename=composer | ||
ENV PATH $COMPOSER_HOME/vendor/bin:$PATH | ||
RUN ln -s $(composer config --global home) /root/composer |
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.
I'd try to chain this command to the previous RUN
:
RUN curl ... && ln -s ...
5.4/Dockerfile
Outdated
RUN curl -sS https://getcomposer.org/installer | php -- --install-dir=/usr/local/bin --filename=composer | ||
ENV PATH $COMPOSER_HOME/vendor/bin:$PATH | ||
RUN ln -s $(composer config --global home) /root/composer | ||
ENV PATH /root/composer/vendor/bin:$PATH |
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.
I agree with @Nemo64 that it would be better to have $PATH:/root/composer/vendor/bin
, even if it used to be this way before.
Tomorrow morning I'll do the changes |
Done |
Thank you @Slamdunk ! Sorry if it took such a long time for me to get this merged. |
Reference: #31 (comment)