Skip to content

Conversation

@cjihrig
Copy link
Contributor

Currently, there is a check to ensure that the user either provides an object or a string to repl.start(). The string case is used to set a REPL prompt. However, a default of '> ' already exists, so forcing the user to specify a prompt is a bit redundant. This commit removes this restriction.

Closes#5385

@mscdexmscdex added the repl Issues and PRs related to the REPL subsystem. label Feb 23, 2016
@bnoordhuis
Copy link
Member

LGTM

@cjihrig
Copy link
ContributorAuthor

What is the game plan for landing code changes while Jenkins is offline? Should we just wait until it's back to use the normal CI flow?

@bnoordhuis
Copy link
Member

I think that would be best.

@Fishrock123Fishrock123 added the semver-minor PRs that contain new features and should be released in the next minor version. label Feb 23, 2016
@Fishrock123
Copy link
Contributor

LGTM, we should be holding for CI I think.

@julianduque
Copy link
Contributor

LGTM with a minor nit, doc should be updated to specify options as optional argument: https://github.com/nodejs/node/blob/master/doc/api/repl.markdown#replstartoptions

Currently, there is a check to ensure that the user either provides an object or a string to repl.start(). The string case is used to set a REPL prompt. However, a default of '> ' already exists, so forcing the user to specify a prompt is a bit redundant. This commit removes this restriction. Fixes: nodejs#5385 PR-URL: nodejs#5388 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Julian Duque <[email protected]>
@cjihrig
Copy link
ContributorAuthor

Addressed @julianduque comment. CI is green - https://ci.nodejs.org/job/node-test-pull-request/1748/. Landing. Thanks for the reviews.

@cjihrigcjihrig merged commit ee7754b into nodejs:masterFeb 25, 2016
@cjihrigcjihrig deleted the 5385 branch February 25, 2016 14:24
This was referenced Mar 1, 2016
Fishrock123 pushed a commit that referenced this pull request Mar 8, 2016
Currently, there is a check to ensure that the user either provides an object or a string to repl.start(). The string case is used to set a REPL prompt. However, a default of '> ' already exists, so forcing the user to specify a prompt is a bit redundant. This commit removes this restriction. Fixes: #5385 PR-URL: #5388 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Julian Duque <[email protected]>
Fishrock123 pushed a commit that referenced this pull request Mar 8, 2016
Currently, there is a check to ensure that the user either provides an object or a string to repl.start(). The string case is used to set a REPL prompt. However, a default of '> ' already exists, so forcing the user to specify a prompt is a bit redundant. This commit removes this restriction. Fixes: #5385 PR-URL: #5388 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Julian Duque <[email protected]>
Fishrock123 added a commit that referenced this pull request Mar 8, 2016
Notable changes: * child_process: “send()” now accepts an options parameter (cjihrig) #5283 - Currently the only option is “keepOpen”, which keeps the underlying socket open after the message is sent. * constants: “ENGINE_METHOD_RSA” is now correctly exposed (Sam Roberts) #5463 * Fixed two regressions which originated in v5.7.0: - http: Errors inside of http client callbacks now propagate correctly (Trevor Norris) #5591 - path: Fixed normalization of absolute paths (Evan Lucas) #5589 * repl: “start()” no longer requires an options parameter (cjihrig) #5388 * util: Improved “format()” performance 50-300% (Evan Lucas) #5360 PR-URL: #5559
Fishrock123 added a commit to Fishrock123/node that referenced this pull request Mar 9, 2016
Notable changes: * child_process: “send()” now accepts an options parameter (cjihrig) nodejs#5283 - Currently the only option is “keepOpen”, which keeps the underlying socket open after the message is sent. * constants: “ENGINE_METHOD_RSA” is now correctly exposed (Sam Roberts) nodejs#5463 * Fixed two regressions which originated in v5.7.0: - http: Errors inside of http client callbacks now propagate correctly (Trevor Norris) nodejs#5591 - path: Fixed normalization of absolute paths (Evan Lucas) nodejs#5589 * repl: “start()” no longer requires an options parameter (cjihrig) nodejs#5388 * util: Improved “format()” performance 50-300% (Evan Lucas) nodejs#5360 PR-URL: nodejs#5559
@TrottTrott mentioned this pull request May 18, 2016
2 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

replIssues and PRs related to the REPL subsystem.semver-minorPRs that contain new features and should be released in the next minor version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@cjihrig@bnoordhuis@Fishrock123@julianduque@mscdex