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

doc: add notice about useGlobal option in repl docs #13866

Closed
wants to merge 1 commit into from
Closed

doc: add notice about useGlobal option in repl docs #13866

wants to merge 1 commit into from

Conversation

starkwang
Copy link
Contributor

Add notice about the useGlobal option in repl docs.

Fixes: #13827

Checklist
Affected core subsystem(s)

doc

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. repl Issues and PRs related to the REPL subsystem. labels Jun 22, 2017
cjihrig
cjihrig approved these changes Jun 22, 2017
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.

LGTM with a suggestion.

doc/api/repl.md Outdated
@@ -406,6 +406,8 @@ changes:
* `useGlobal` {boolean} If `true`, specifies that the default evaluation
function will use the JavaScript `global` as the context as opposed to
creating a new separate context for the REPL instance. Defaults to `false`.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be adequate to say:

The CLI REPL sets this value to true. Defaults to false when used programmatically.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the key point is to make readers notice the default value is false, while node CLI REPL use true in comparison.
Your suggestion is good. I try to make it less wordy.

@gibfahn gibfahn mentioned this pull request Jul 13, 2017
Closed
jasnell pushed a commit that referenced this pull request Jul 14, 2017
@starkwang @jasnell
doc: add notice about useGlobal option in repl docs
6e47e13 
PR-URL: #13866
Fixes: #13827
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@jasnell
Copy link
Member

jasnell commented Jul 14, 2017

Landed in 6e47e13

@jasnell jasnell closed this Jul 14, 2017
@addaleax addaleax mentioned this pull request Jul 18, 2017
Merged
addaleax pushed a commit that referenced this pull request Jul 18, 2017
@starkwang @addaleax
doc: add notice about useGlobal option in repl docs
157ef23 
PR-URL: #13866
Fixes: #13827
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Fishrock123 pushed a commit that referenced this pull request Jul 19, 2017
@starkwang @Fishrock123
doc: add notice about useGlobal option in repl docs
361c35e 
PR-URL: #13866
Fixes: #13827
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gibfahn gibfahn gibfahn approved these changes

@cjihrig cjihrig cjihrig approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
doc Issues and PRs related to the documentations. repl Issues and PRs related to the REPL subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@starkwang @jasnell @cjihrig @gibfahn @MylesBorins @nodejs-github-bot