Skip to content

Add Node.js v8.0.0 #411

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 1 commit into from
May 31, 2017
Merged

Add Node.js v8.0.0 #411

merged 1 commit into from
May 31, 2017

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented May 30, 2017

#410 with node 7 removed. Unsure which of these (if any) you want 😀

@SimenB SimenB mentioned this pull request May 30, 2017
Closed
@chorrell
Copy link
Contributor

The Docker files should be in a 8.0/ directory.

@SimenB
Copy link
Member Author

SimenB commented May 30, 2017

The Docker files should be in a 8.0/ directory.

They are.
image

@chorrell
Copy link
Contributor

Ha! I missed that.

In that case LGTM

@SimenB
Copy link
Member Author

SimenB commented May 30, 2017

You should enable cancelling old builds on new pushes on travis. I force pushed a bit, and the queue is real right now 😭

https://blog.travis-ci.com/2017-03-22-introducing-auto-cancellation

@chorrell
Copy link
Contributor

I would if I had the access to do that ;-)

Someone from the build group should be able to enable that for us.

@SimenB
Copy link
Member Author

SimenB commented May 30, 2017

Should I close #410?

@chorrell
Copy link
Contributor

Sure.

@pouwerkerk pouwerkerk mentioned this pull request May 30, 2017
Closed
Starefossen

This comment was marked as off-topic.

# to view

@pesho
Copy link
Contributor

pesho commented May 30, 2017

Wait, is 7.x a completely abandoned branch now?

@SimenB
Copy link
Member Author

SimenB commented May 30, 2017 •
edited
Loading

Wait, is 7.x a completely abandoned branch now?

https://github.com/nodejs/LTS/blob/4af640885c5888894348ace2b15942e8989494e4/README.md

An odd-numbered major release will cease to be actively updated when the subsequent even-numbered major release is cut.

Not really sure if that means "completely abandoned", though...

Btw, can one of you cancel some of the builds here? https://travis-ci.org/nodejs/docker-node/pull_requests (bonus is activating auto cancellation 🙂)

@pesho
Copy link
Contributor

pesho commented May 30, 2017

@SimenB thanks, it is indeed abandoned then.

I think the Yarn version should be updated too in this PR, according to the (not yet officially accepted) policy in #393

@chorrell
Copy link
Contributor

Were we also going to update Alpine for 8.x or was there a different plan for that? Either way, that can probably be handled separately.

@pesho
Copy link
Contributor

pesho commented May 30, 2017

Yes, Alpine should also be updated (probably to 3.6) according to the discussion in #399

@SimenB
Copy link
Member Author

SimenB commented May 30, 2017

I think the Yarn version should be updated too in this PR

I chose not to update it to keep it consistent between images. That doesn't really make sense though, the npm version is already different. Want me to update it?

@Starefossen
Copy link
Member

Starefossen commented May 30, 2017 •
edited
Loading

I'd vote we merge this PR as is and make a separate ones for upgrade of Alpine and Yarn.

@pesho
Copy link
Contributor

pesho commented May 30, 2017

I chose not to update it to keep it consistent between images.

Running ./update.sh 8.0 should "Do the right thing":tm: and only update it for 8.x.

@pesho
Copy link
Contributor

pesho commented May 30, 2017

I'm ok with doing Yarn and Alpine separately in this case, but in general think they should be handled in a single PR, atomically.

@SimenB
Copy link
Member Author

SimenB commented May 30, 2017

CI is green, but it doesn't seem connected to the latest commit in this PR... https://travis-ci.org/nodejs/docker-node/builds/237656473

@pouwerkerk
Copy link

@SimenB looks like the commit that CI built is the merge commit: 8e936c8

@olalonde
Copy link

💤

@xzyfer xzyfer mentioned this pull request May 31, 2017
Closed
6 tasks
@SimenB
Copy link
Member Author

SimenB commented May 31, 2017 •
edited
Loading

CI is confused, I force pushed the same commit now.

EDIT: Didn't help at all... I have no idea, just merge it? The same diff was green even if github/travis is a bit confused

@lostintangent lostintangent mentioned this pull request May 31, 2017
Merged
@Starefossen
Copy link
Member

I checked locally, it's all good. Merging.

@Starefossen Starefossen merged commit 82c21d3 into nodejs:master May 31, 2017
@SimenB SimenB deleted the node-8-del-7 branch May 31, 2017 06:04
@SimenB
Copy link
Member Author

SimenB commented May 31, 2017 •
edited
Loading

Great! Will this be pushed to the docker registry, or should yarn and alpine be updated first?

EDIT: PR for the former: #412, latter: #413

@Starefossen
Copy link
Member

Starefossen commented May 31, 2017 •
edited
Loading

Thanks a lot @SimenB. I'll merge them as soon as the builds finish. I have prepared a PR to Docker Hub. Hopefully will get this one out buy by the end of the day.

This was referenced May 31, 2017
Merged
Merged
tianon pushed a commit to docker-library/official-images that referenced this pull request May 31, 2017
Upgrade Node to 8.0.0
34aeba6 
PR-URL #2997

Related: nodejs/node#12220 nodejs/docker-node#411 nodejs/docker-node#412 nodejs/docker-node#113

Signed-off-by: Hans Kristian Flaatten <hans@starefossen.com>
@dy-dx dy-dx mentioned this pull request Jul 11, 2017
Closed
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@Starefossen Starefossen Starefossen approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@SimenB @chorrell @pesho @Starefossen @pouwerkerk @olalonde @analytically @LaurentGoderre