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: fix variable scoping bug in Stream HTTP server example code #8124

Closed

Conversation

lazlojuly
Copy link
Contributor

@lazlojuly lazlojuly commented Aug 16, 2016

Checklist
  • documentation is changed or added
  • commit message follows commit guidelines
Description of change

Const is block-scoped.
Stream HTTP server example code is broken since const was introduced inside a try block.

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. stream Issues and PRs related to the stream subsystem. labels Aug 16, 2016
@@ -122,23 +122,22 @@ const server = http.createServer( (req, res) => {
req.setEncoding('utf8');

// Readable streams emit 'data' events once a listener is added
req.on('data', (chunk) => {
req.on('data', chunk => {
Copy link
Member

Choose a reason for hiding this comment

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

nit: our preferred code style in core is to require parens around arguments in arrow functions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed (n)it

@jasnell
Copy link
Member

jasnell commented Aug 16, 2016

LGTM with a nit

@lazlojuly lazlojuly force-pushed the doc-stream-http-example-fix branch from 6b07664 to 061880f Compare August 16, 2016 17:34
@MylesBorins
Copy link
Contributor

LGTM

jasnell pushed a commit that referenced this pull request Aug 18, 2016
Const is block scoped.

PR-URL: #8124
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
@jasnell
Copy link
Member

jasnell commented Aug 18, 2016

Landed in 66d697c! thank you @lazlojuly !

@jasnell jasnell closed this Aug 18, 2016
evanlucas pushed a commit that referenced this pull request Aug 24, 2016
Const is block scoped.

PR-URL: #8124
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 30, 2016
Const is block scoped.

PR-URL: #8124
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 10, 2016
Const is block scoped.

PR-URL: #8124
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
rvagg pushed a commit that referenced this pull request Oct 18, 2016
Const is block scoped.

PR-URL: #8124
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 26, 2016
Const is block scoped.

PR-URL: #8124
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Oct 26, 2016
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. stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants