Conversation
23e185e to
6f912d8
Compare
8a8fb6b to
6fd8ef8
Compare
quartje
left a comment
There was a problem hiding this comment.
Nice work. I like the idea of a general build container that is able to build all OpenConext projects.
I'd be nice if we don't have to put specific version numbers in this container, that makes it quickly outdated
| password: ${{ secrets.GITHUB_TOKEN }} | ||
|
|
||
| - name: Build the Apache container and push to GitHub Packages | ||
| - name: Build the Stepup build container and push to GitHub Packages |
There was a problem hiding this comment.
Would this be usable for EB as well? Makes maybe sense to have only one container for all build purposes
There was a problem hiding this comment.
I guess this should work with EB just fine. Certainly opportune to test this!
There was a problem hiding this comment.
One container to rule them all!
| ENV NODE_VERSION 10 | ||
|
|
||
| # Install nvm with node and npm | ||
| RUN curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.37.2/install.sh | bash |
There was a problem hiding this comment.
Alltough this syntax works, it's more common to put && \ at the end of the line and start the newline with the next command
| RUN curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.37.2/install.sh | bash | ||
| && source $NVM_DIR/nvm.sh \ | ||
| RUN mkdir $NVM_DIR \ | ||
| && curl -o- https://raw.githubusercontent.com/creationix/nvm/v0.39.0/install.sh | bash \ |
There was a problem hiding this comment.
Is there no more generic way than a hardcoded version to download?
There was a problem hiding this comment.
I will look if they provide a latest or stable variety.
There was a problem hiding this comment.
Sadly there is no possibility to do this at this point.
docker/php-build-stepup/Dockerfile
Outdated
| # Yarn | ||
| && npm install --global yarn \ | ||
| # Link Node and Yarn to be available using Docker Run | ||
| && ln -s /usr/local/nvm/versions/node/v10.24.1/bin/node /usr/local/bin/node \ |
There was a problem hiding this comment.
Is there no more generic way than a hardcoded version link? This will get outdated quickly
There was a problem hiding this comment.
Yes, I ended up with exporting the node version to an env-var. And using that instead. That should work out
| @@ -1,8 +1,5 @@ | |||
| FROM php:7.2 | |||
There was a problem hiding this comment.
Giving that a shot, It should be..
ee41084 to
9989a94
Compare
9989a94 to
d23942e
Compare
We will be using the binaries from a bash standpoint, no need to expose these for us them using docker run
89ade2b to
e5fcd0b
Compare
This is the continuation of #3 That PR wasn't reviewed, and that should be done in this effort.
What changed here is:
Using
docker run, this container exposesphp,yarnandcomposer