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

install yarn from apt repo #11

Merged
merged 2 commits into from Jan 27, 2017
Merged

install yarn from apt repo #11

merged 2 commits into from Jan 27, 2017

Conversation

pklingem
Copy link
Member

we should consider installing yarn from the apt repo as the yarn maintainers suggest. It feels wrong to install via npm anyway. The only downside is that we can't lock the version down if we want to. @articulate/ops what are your thoughts?

@plukevdh
Copy link
Contributor

@tecnobrat
Copy link
Contributor

tecnobrat commented Jan 26, 2017

You actually can lock down the version.

Change your apt-get install line to look like: apt-get install yarn=0.19.1-1

Also, you should break this out into it a separate RUN line:

RUN apt-get update && \
    apt-get install apt-transport-https

RUN curl -sS https://dl.yarnpkg.com/debian/pubkey.gpg | apt-key add - && \
    echo "deb https://dl.yarnpkg.com/debian/ stable main" | tee /etc/apt/sources.list.d/yarn.list && \
    apt-get update && \
    apt-get install yarn=0.19.1-1 && \
    apt-get clean

The reason is that docker will cache those separately now, which will speed up the build when all you wanna do is bump the version of yarn. Otherwise you do a apt-get update twice, for no reason.

I'm also going to suggest leaving the consul template lines directly below the FROM node:7 line, again for caching sake. Then when you bump yarn it doesn't need to redo the consul stuff after, it'll be cached from the previous run.

@pklingem
Copy link
Member Author

@tecnobrat good thoughts, I will address those. I wish there was a nice way to prevent the duplication across those Dockerfiles, but I can't think of one.

@pklingem
Copy link
Member Author

addressed @tecnobrat's comments, merging.

@pklingem pklingem merged commit 7bf0c32 into master Jan 27, 2017
@pklingem pklingem deleted the yarn-apt-repo branch January 27, 2017 14:34
@pklingem pklingem mentioned this pull request Jan 27, 2017
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.

None yet

3 participants