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

repl: hide top-level await feature behind a flag #19604

Closed
wants to merge 5 commits into from
Closed

repl: hide top-level await feature behind a flag #19604

wants to merge 5 commits into from

Conversation

TimothyGu
Copy link
Member

Refs: #17807
Refs: #15566 (comment)

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

@TimothyGu TimothyGu requested a review from bmeck March 26, 2018 03:23
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. repl Issues and PRs related to the REPL subsystem. labels Mar 26, 2018
@vsemozhetbyt
Copy link
Contributor

Should this be documented?

@vsemozhetbyt vsemozhetbyt added the promises Issues and PRs related to ECMAScript promises. label Mar 26, 2018
@TimothyGu
Copy link
Member Author

TimothyGu commented Mar 29, 2018 •
edited
Loading

@vsemozhetbyt Done.

CI: https://ci.nodejs.org/job/node-test-commit/17264/

vsemozhetbyt
vsemozhetbyt reviewed Mar 29, 2018
View reviewed changes
doc/api/repl.md Outdated
> await Promise.reject(new Error('REPL await'))
Error: REPL await
at repl:1:45
> timeout = util.promisify(setTimeout)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: maybe const timeout = and const old = to not emphasize sloppy mode?

@TimothyGu
Copy link
Member Author

TimothyGu commented Apr 14, 2018 •
edited
Loading

Rebased. @bmeck, @vsemozhetbyt PTAL.

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

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

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

@TimothyGu
Copy link
Member Author

I'll be landing this in a few days.

TimothyGu added a commit that referenced this pull request Apr 18, 2018
@TimothyGu
repl: hide top-level await feature behind a flag
be92eab 
PR-URL: #19604
Refs: #17807
Refs: #15566 (comment)
Reviewed-By: Gus Caplan <me@gus.host>
@TimothyGu
Copy link
Member Author

Landed in be92eab.

@jasnell, this needs to go into 10.0.0 to alleviate @bmeck's concern.

@TimothyGu TimothyGu closed this Apr 18, 2018
@TimothyGu TimothyGu deleted the repl-await-flag branch April 18, 2018 15:18
@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Apr 18, 2018 •
edited
Loading

@TimothyGu It seems the flag also should be mentioned in doc/api/cli.md and doc/node.1.
I am sorry I missed this( Excuse me for being too late.

@jasnell jasnell added this to the 10.0.0 milestone Apr 18, 2018
@jasnell
Copy link
Member

jasnell commented Apr 18, 2018

Since this is semver-patch it will definitely make it in to 10.0.0

@TimothyGu
Copy link
Member Author

@vsemozhetbyt See #20133.

@TimothyGu TimothyGu mentioned this pull request Apr 18, 2018
Closed
3 tasks
@vsemozhetbyt
Copy link
Contributor

This should be backported with #20133

TimothyGu added a commit that referenced this pull request Apr 18, 2018
@TimothyGu
doc: fully document --experimental-repl-await
ceab78f 
PR-URL: #20133
Fixes: #19604 (comment)
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
jasnell pushed a commit that referenced this pull request Apr 19, 2018
@TimothyGu @jasnell
repl: hide top-level await feature behind a flag
f2396ee 
PR-URL: #19604
Refs: #17807
Refs: #15566 (comment)
Reviewed-By: Gus Caplan <me@gus.host>
jasnell pushed a commit that referenced this pull request Apr 19, 2018
@TimothyGu @jasnell
doc: fully document --experimental-repl-await
56d6e82 
PR-URL: #20133
Fixes: #19604 (comment)
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Krinkle Krinkle mentioned this pull request May 2, 2018
Closed
@webmaster128 webmaster128 mentioned this pull request Jul 21, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vsemozhetbyt vsemozhetbyt vsemozhetbyt left review comments

@devsnek devsnek devsnek approved these changes

@bmeck bmeck Awaiting requested review from bmeck

Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. promises Issues and PRs related to ECMAScript promises. repl Issues and PRs related to the REPL subsystem.
Projects
None yet
Milestone
10.0.0
Development

Successfully merging this pull request may close these issues.
5 participants
@TimothyGu @vsemozhetbyt @jasnell @devsnek @nodejs-github-bot