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

feat: Node v13.11.0 and Yarn 1.22.4 #1229

Closed
wants to merge 2 commits into from

Conversation

nschonni
Copy link
Member

@nschonni nschonni commented Mar 12, 2020

There is currently an issue with the nodejs.org website webhook, but the release post has landed

13/alpine3.10/Dockerfile Outdated Show resolved Hide resolved
@nschonni nschonni marked this pull request as ready for review March 12, 2020 05:25
@nschonni nschonni requested a review from a team March 12, 2020 05:26
@@ -74,9 +74,6 @@ for version in "${versions[@]}"; do
tag=$(get_tag "${version}")
full_version=$(get_full_version "${version}")

build "${version}" "default" "${tag}"
test_image "${full_version}" "default" "${tag}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this fix for?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My Chakracore fix in the other PR was incorrect in removing just the if and leaving the contents. It should have removed these lines, so when I pushed the original PR, the build failed on the tests.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's actually a little clearer on my GH Actions PR #1194 where all the builds failed before doing the same fix

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's separate the fix to another PR, it has nothing to do with the version update itself.

Copy link
Member

@PeterDaveHello PeterDaveHello left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duplicated review

Copy link
Member

@PeterDaveHello PeterDaveHello left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if the fix commit is for the build https://travis-ci.com/github/nodejs/docker-node/builds/152927756 which was failed originally, I restarted the builds and the all passed now.

@Congelli501
Copy link

Hey, is there any chance this will be merged soon ?
Node 13.10 breaks stuff (nodejs/node#32105) that node 13.11 fixes.

@SimenB
Copy link
Member

SimenB commented Mar 25, 2020

I cannot merge this since it has a requested change. @PeterDaveHello @nschonni can you update?

image

@PeterDaveHello
Copy link
Member

The unrelated commit should be separated, I'll help do that.

@PeterDaveHello
Copy link
Member

See #1233

@nschonni nschonni closed this Mar 26, 2020
@nschonni nschonni deleted the node-13.11.0-yarn-1.22.4 branch March 26, 2020 13:55
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

4 participants