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

doc: some fixes in repl.md #10244

Closed
wants to merge 6 commits into from
Closed

doc: some fixes in repl.md #10244

wants to merge 6 commits into from

Conversation

vsemozhetbyt
Copy link
Contributor

@vsemozhetbyt vsemozhetbyt commented Dec 13, 2016

Checklist
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc, repl

Description of change
  1. var => let / const.

  2. White space unification (add an infix space in an argument list; change > into > (with a trailing space) in code bits and output examples; explicitly clarify that default REPL prompt contains a trailing space).

  3. Fix an output example (make _ reassignment example match with the current output; extend the example for more clarity).

  4. Fix a function name (eval => myEval to not shadow the global eval).

  5. Replace anonymous functions with an object shorthand and an arrow function.

  6. Add the valid link for curl(1) (The current autoinserted link leads to 404 page).

I am sorry I've messed up the previous PR by an inexperienced rebasing during resolving a conflict. Excuse me for wasting your time.

Add an infix space in an argument list.
Change `>` into `> ` in code bits and output examples.
Explicitly clarify that default REPL prompt contains a trailing space.
Make `_` reassignment example match more with the current output.
Extend the example for more clarity.
`eval` => `myEval` to not shadow the global `eval`
Replaced with an object shorthand and an arrow function.
@nodejs-github-bot nodejs-github-bot added dont-land-on-v7.x doc Issues and PRs related to the documentations. repl Issues and PRs related to the REPL subsystem. labels Dec 13, 2016
@vsemozhetbyt vsemozhetbyt changed the title Repl.md doc: some fixes in repl.md Dec 13, 2016
@vsemozhetbyt
Copy link
Contributor Author

/cc @nodejs/documentation

@vsemozhetbyt
Copy link
Contributor Author

@addaleax, @eljefedelrodeodeljefe, @lance, you had reviewed the previous ill-fated PR. If anybody has the time, could you please look at this? Sorry for the disturbing.

Copy link
Member

@addaleax addaleax 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 grammar nit

declared either implicitly or using the `var` keyword are declared at the
`global` scope.
Unless otherwise scoped within blocks or functions, variables declared
either implicitly, or using the `const`, `let`, or `var` keywords
Copy link
Member

Choose a reason for hiding this comment

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

The , here doesn’t seem quite right to me?

Copy link
Contributor Author

@vsemozhetbyt vsemozhetbyt Dec 17, 2016

Choose a reason for hiding this comment

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

@addaleax Do you mean "let, or var"? It is a @lance's sentence. It seems he uses Oxford comma here. Should I change it to "let or var"?

Or do you mean another comma?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, sorry. I mean the one after implicitly… the Oxford comma after let seems okay here :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Amended with the last commit. Thank you :)

The current autoinserted link leads to 404 page.
@thefourtheye
Copy link
Contributor

cc @princejwesley

@vsemozhetbyt
Copy link
Contributor Author

vsemozhetbyt commented Dec 25, 2016

@addaleax, @jasnell, @thefourtheye Could this be landed?

addaleax pushed a commit that referenced this pull request Dec 25, 2016
PR-URL: #10244
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: James M Snell <[email protected]>
addaleax pushed a commit that referenced this pull request Dec 25, 2016
Add an infix space in an argument list.
Change `>` into `> ` in code bits and output examples.
Explicitly clarify that default REPL prompt contains a trailing space.

PR-URL: #10244
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: James M Snell <[email protected]>
addaleax pushed a commit that referenced this pull request Dec 25, 2016
Make `_` reassignment example match more with the current output.
Extend the example for more clarity.

PR-URL: #10244
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: James M Snell <[email protected]>
addaleax pushed a commit that referenced this pull request Dec 25, 2016
`eval` => `myEval` to not shadow the global `eval`

PR-URL: #10244
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: James M Snell <[email protected]>
addaleax pushed a commit that referenced this pull request Dec 25, 2016
Replaced with an object shorthand and an arrow function.

PR-URL: #10244
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: James M Snell <[email protected]>
addaleax pushed a commit that referenced this pull request Dec 25, 2016
The current autoinserted link leads to 404 page.

PR-URL: #10244
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@addaleax
Copy link
Member

@vsemozhetbyt Thanks for the ping!

Landed in 45c9ca7...f501178.

@addaleax addaleax closed this Dec 25, 2016
@vsemozhetbyt vsemozhetbyt deleted the repl.md branch December 25, 2016 18:55
evanlucas pushed a commit that referenced this pull request Jan 3, 2017
PR-URL: #10244
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: James M Snell <[email protected]>
evanlucas pushed a commit that referenced this pull request Jan 3, 2017
Add an infix space in an argument list.
Change `>` into `> ` in code bits and output examples.
Explicitly clarify that default REPL prompt contains a trailing space.

PR-URL: #10244
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: James M Snell <[email protected]>
evanlucas pushed a commit that referenced this pull request Jan 3, 2017
Make `_` reassignment example match more with the current output.
Extend the example for more clarity.

PR-URL: #10244
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: James M Snell <[email protected]>
evanlucas pushed a commit that referenced this pull request Jan 3, 2017
`eval` => `myEval` to not shadow the global `eval`

PR-URL: #10244
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: James M Snell <[email protected]>
evanlucas pushed a commit that referenced this pull request Jan 3, 2017
Replaced with an object shorthand and an arrow function.

PR-URL: #10244
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: James M Snell <[email protected]>
evanlucas pushed a commit that referenced this pull request Jan 3, 2017
The current autoinserted link leads to 404 page.

PR-URL: #10244
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: James M Snell <[email protected]>
evanlucas pushed a commit that referenced this pull request Jan 4, 2017
PR-URL: #10244
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: James M Snell <[email protected]>
evanlucas pushed a commit that referenced this pull request Jan 4, 2017
Add an infix space in an argument list.
Change `>` into `> ` in code bits and output examples.
Explicitly clarify that default REPL prompt contains a trailing space.

PR-URL: #10244
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: James M Snell <[email protected]>
evanlucas pushed a commit that referenced this pull request Jan 4, 2017
Make `_` reassignment example match more with the current output.
Extend the example for more clarity.

PR-URL: #10244
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: James M Snell <[email protected]>
evanlucas pushed a commit that referenced this pull request Jan 4, 2017
`eval` => `myEval` to not shadow the global `eval`

PR-URL: #10244
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: James M Snell <[email protected]>
evanlucas pushed a commit that referenced this pull request Jan 4, 2017
Replaced with an object shorthand and an arrow function.

PR-URL: #10244
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: James M Snell <[email protected]>
evanlucas pushed a commit that referenced this pull request Jan 4, 2017
The current autoinserted link leads to 404 page.

PR-URL: #10244
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@MylesBorins
Copy link
Contributor

Would someone be willing to manually backport these changes to v6.x?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. repl Issues and PRs related to the REPL subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants