Skip to content

Conversation

@itajaja
Copy link

@itajaja itajaja commented Jun 1, 2017

fixes #419

disclaimer: untested

pesho

This comment was marked as off-topic.

@pesho pesho dismissed their stale review June 2, 2017 10:44

Actually this should be fixed in the template.

@itajaja
Copy link
Author

itajaja commented Jun 5, 2017

uh, how? is that file generated?

@Starefossen
Copy link
Member

It is locally generated before pushing, the template lives in the root folder: Dockerfile-onbuild.template

@itajaja
Copy link
Author

itajaja commented Jun 5, 2017

I'd argue tho that this only makes sense for node 8

@pesho
Copy link
Contributor

pesho commented Jun 5, 2017

The same template generates all dockerfiles, so the Node 8 one will be overwritten in future releases unless the template is updated. The --force param is backwards-compatible.

@itajaja
Copy link
Author

itajaja commented Jun 5, 2017

I'll put it in the template then 👍

@Starefossen
Copy link
Member

Starefossen commented Jun 6, 2017

Could you rebase your branch on master?

git fetch origin/maseter
git rebase --no-ff origin/master

And then you'll need to force push to your forked branch.

@gibfahn
Copy link
Member

gibfahn commented Jun 6, 2017

I'm confused, I can't see a change to the npm cache clean line. Am I am I missing something?

@itajaja
Copy link
Author

itajaja commented Jun 7, 2017

@gibfahn that links points to master, this pr hasn't been merged yet :)

@itajaja
Copy link
Author

itajaja commented Jun 7, 2017

@Starefossen rebased 🙌

@pesho pesho mentioned this pull request Jun 7, 2017
@pesho
Copy link
Contributor

pesho commented Jun 7, 2017

@itajaja the PR is still broken, closing. I've pushed #426 which is a proper fix.

@pesho pesho closed this Jun 7, 2017
@itajaja
Copy link
Author

itajaja commented Jun 7, 2017

sorry, don't know what happened, I just rebased in a rush and I probably messed something

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.

npm cache clean throws with npm 5

4 participants