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

Revert: repl: support top-level await #17807

Closed
wants to merge 5 commits into from
Closed

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Dec 21, 2017

This reverts #15566

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

deps, test, repl

@nodejs-github-bot nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Dec 21, 2017
@cjihrig cjihrig mentioned this pull request Dec 21, 2017
4 tasks
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

LGTM

@jasnell
Copy link
Member

jasnell commented Dec 21, 2017

I'm +1 on this but I would also be plus one on keeping the new functionality as experimental and keeping it behind a flag while the issues are worked through. Top level await is quite popular and users want it.

@cjihrig
Copy link
Contributor Author

cjihrig commented Dec 21, 2017

Yes, I'm definitely in favor of having top level await. I put this together to give us one option.

@bnoordhuis
Copy link
Member

Commit logs need more context. I, for one, have no idea why it's being reverted.

Copy link
Member

@TimothyGu TimothyGu left a comment

Choose a reason for hiding this comment

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

-1 from me because of #15566 (comment). I’d rather see it being hidden behind a flag, an approach okay with @bmeck in #15566 (comment).

@mcollina
Copy link
Member

I’m ok for a flag too.

@cjihrig
Copy link
Contributor Author

cjihrig commented Dec 21, 2017

A flag it is then!

@cjihrig cjihrig closed this Dec 21, 2017
@cjihrig cjihrig deleted the revert branch December 21, 2017 21:47
TimothyGu added a commit to TimothyGu/node that referenced this pull request Apr 14, 2018
TimothyGu added a commit that referenced this pull request Apr 18, 2018
jasnell pushed a commit that referenced this pull request Apr 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants