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

[12.x] module: self resolve flag as experimental modules #31757

Closed
wants to merge 1 commit into from
Closed

[12.x] module: self resolve flag as experimental modules #31757

wants to merge 1 commit into from

Conversation

guybedford
Copy link
Contributor

@guybedford guybedford commented Feb 12, 2020 •
edited
Loading

Resolves #31754.

Ensure package self resolution remains behind --experimental-modules.

//cc @nodejs/modules-active-members @MylesBorins

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@guybedford guybedford changed the title module: 12.x self resolve flag as experimental modules [12.x] module: self resolve flag as experimental modules Feb 12, 2020
ljharb
ljharb approved these changes Feb 12, 2020
View reviewed changes
Copy link
Member

@ljharb ljharb left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks! (edit: oops, i'm not a collaborator; ignore the green color on this checkmark)

@ljharb ljharb added esm Issues and PRs related to the ECMAScript Modules implementation. module Issues and PRs related to the module subsystem. labels Feb 12, 2020
@jkrems
Copy link
Contributor

jkrems commented Feb 12, 2020

Should the base branch be changed to v12.x-staging? See: https://github.com/nodejs/node/blob/master/COLLABORATOR_GUIDE.md#how-are-lts-branches-managed

@guybedford guybedford changed the base branch from v12.x to v12.x-staging February 13, 2020 11:56
MylesBorins
MylesBorins approved these changes Feb 13, 2020
View reviewed changes
Copy link
Contributor

@MylesBorins MylesBorins left a comment

Choose a reason for hiding this comment

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

LGTM

I'll try and get something together this afternoon

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 13, 2020 •
edited by MylesBorins
Loading

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

PPC retry: https://ci.nodejs.org/job/node-test-commit-plinux/30916/
arm retry: https://ci.nodejs.org/job/node-test-binary-arm-12+/4342/

MylesBorins pushed a commit that referenced this pull request Feb 13, 2020
@guybedford @MylesBorins
module: 12.x self resolve flag as experimental modules
f0b2d87 
PR-URL: #31757
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
@MylesBorins
Copy link
Contributor

landed in f0b2d87

@MylesBorins MylesBorins mentioned this pull request Feb 13, 2020
Closed
@ljharb
Copy link
Member

ljharb commented Feb 13, 2020

Seems like the commit was missing the "resolves" stanza that auto-closes the linked issue?

@guybedford guybedford mentioned this pull request Apr 21, 2020
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jkrems jkrems jkrems approved these changes

@MylesBorins MylesBorins MylesBorins approved these changes

@GeoffreyBooth GeoffreyBooth GeoffreyBooth approved these changes

@ljharb ljharb ljharb approved these changes

Assignees
No one assigned
Labels
esm Issues and PRs related to the ECMAScript Modules implementation. module Issues and PRs related to the module subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@guybedford @jkrems @nodejs-github-bot @MylesBorins @ljharb @GeoffreyBooth