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.

Sign up for GitHub

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

Jump to bottom

test: fix ESM test [Unbreak master] #21605

Closed
wants to merge 1 commit into from
Closed

test: fix ESM test [Unbreak master] #21605

wants to merge 1 commit into from

Conversation

addaleax
Copy link
Member

The previously landed commit was broken and it’s too late
to force-push. Fixing up the test seems to work.

Refs: #21352

@addaleax
test: fix ESM test
5b32ea9 
The previously landed commit was broken and it’s too late
to force-push. Fixing up the test seems to work.

Refs: nodejs#21352
@nodejs-github-bot
Copy link
Collaborator

@addaleax build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/184/pipeline

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Jun 30, 2018
@addaleax addaleax added esm Issues and PRs related to the ECMAScript Modules implementation. fast-track PRs that do not need to wait for 48 hours to land. labels Jun 30, 2018
@addaleax
Copy link
Member Author

CI: https://ci.nodejs.org/job/node-test-pull-request/15683/

@nodejs/collaborators Please approve fast-tracking here by 👍-ing this comment

@vsemozhetbyt vsemozhetbyt mentioned this pull request Jun 30, 2018
Closed
3 tasks
cjihrig
cjihrig approved these changes Jun 30, 2018
View reviewed changes
Copy link
Contributor

@cjihrig cjihrig left a comment

Choose a reason for hiding this comment

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

Please land this this if it unbreaks master.

@addaleax
Copy link
Member Author

Landed in deff8db

/cc @guybedford @devsnek @jasnell @TimothyGu as the original author/reviewers, please take a look and see whether this test needs more care

@addaleax addaleax closed this Jun 30, 2018
@addaleax addaleax deleted the fix-master branch June 30, 2018 12:15
addaleax added a commit that referenced this pull request Jun 30, 2018
@addaleax
test: fix ESM test
deff8db 
The previously landed commit was broken and it’s too late
to force-push. Fixing up the test seems to work.

Refs: #21352

PR-URL: #21605
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
targos pushed a commit that referenced this pull request Jun 30, 2018
@addaleax @targos
test: fix ESM test
b28fd37 
The previously landed commit was broken and it’s too late
to force-push. Fixing up the test seems to work.

Refs: #21352

PR-URL: #21605
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@guybedford
Copy link
Contributor

Thanks @addaleax I've just compared the the PR, with the merge and the the diff you provided here is exactly the difference between the two, seems something got out of sync during my local process (problem with developing between two machines as Windows is my primary OS currently, need to get that Windows Node build running but vcbuild is breaking for me currently for reasons I haven't been able to fix).

@Trott
Copy link
Member

Trott commented Jun 30, 2018

So the PR had the right code but somehow the wrong code got landed? One more data point for the importance of @nodejs/commit-queue . :-D

@targos targos mentioned this pull request Jul 3, 2018
Merged
@snyk-bot snyk-bot mentioned this pull request Mar 18, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjihrig cjihrig cjihrig approved these changes

@targos targos targos approved these changes

Assignees
No one assigned
Labels
esm Issues and PRs related to the ECMAScript Modules implementation. fast-track PRs that do not need to wait for 48 hours to land. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@addaleax @nodejs-github-bot @guybedford @Trott @targos @cjihrig