FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

doc: fix mistake in http2stream.respondWithFile. by antoine-amara · Pull Request #15501 · nodejs/node · GitHub

/ node Public

doc: fix mistake in http2stream.respondWithFile. - #15501

Closed
antoine-amara wants to merge 1 commit into
nodejs:masterfrom
antoine-amara:doc-#15390-update-responseWithFile
Closed

doc: fix mistake in http2stream.respondWithFile.#15501
antoine-amara wants to merge 1 commit into
nodejs:masterfrom
antoine-amara:doc-#15390-update-responseWithFile

Conversation

antoine-amara commented Sep 20, 2017
edited
Loading

Copy link
Copy Markdown
Contributor

http2stream.respondWithFile api has changed since 8.5.0 with the
addition of the onError option. In the first code example an
onError function is implemented but never used, fix this
mistake.

Add a description more informations when onError is triggered.

Fixes: #15390

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

nodejs-github-bot added doc Issues and PRs related to the documentations. http2 Issues or PRs related to the http2 subsystem. labels Sep 20, 2017
Comment thread doc/api/http2.md Outdated

lpinca Sep 21, 2017
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Nit: can you please add backticks (`) around onError?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Of course, I will fix that and I rebase on master too.

antoine-amara force-pushed the doc-#15390-update-responseWithFile branch from 3959dc9 to 705bbe2 Compare September 21, 2017 11:34

Copy link
Copy Markdown
Contributor Author

Fix the onError syntax and rebase the branch on master. Thanks for the review.

Comment thread doc/api/http2.md Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The send if is defined sounds somewhat wrong to me. Can we change that to something like
If the `onError` callback is defined it will be called, otherwise the stream will be destroyed.?

antoine-amara force-pushed the doc-#15390-update-responseWithFile branch from 705bbe2 to 2200309 Compare September 24, 2017 17:56
http2stream.respondWithFile api has changed since 8.5.0 with the
addition of the onError option. In the first code example an
onError function is implemented but never used, fix this
mistake.

Add a description to have more informations when onError is triggered.

Fixes: nodejs#15390
antoine-amara force-pushed the doc-#15390-update-responseWithFile branch from 2200309 to 42c244d Compare September 26, 2017 11:15

Copy link
Copy Markdown
Member

Landed in 51bc7fa

BridgeAR closed this Sep 27, 2017
BridgeAR pushed a commit that referenced this pull request Sep 27, 2017
http2stream.respondWithFile api has changed since 8.5.0 with the
addition of the onError option. In the first code example an
onError function is implemented but never used, fix this
mistake.

Add a description to have more informations when onError is triggered.

PR-URL: #15501
Fixes: #15390
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Sep 28, 2017
http2stream.respondWithFile api has changed since 8.5.0 with the
addition of the onError option. In the first code example an
onError function is implemented but never used, fix this
mistake.

Add a description to have more informations when onError is triggered.

PR-URL: nodejs#15501
Fixes: nodejs#15390
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
antoine-amara deleted the doc-#15390-update-responseWithFile branch September 28, 2017 08:08
MylesBorins pushed a commit that referenced this pull request Sep 29, 2017
http2stream.respondWithFile api has changed since 8.5.0 with the
addition of the onError option. In the first code example an
onError function is implemented but never used, fix this
mistake.

Add a description to have more informations when onError is triggered.

PR-URL: #15501
Fixes: #15390
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
addaleax pushed a commit to addaleax/ayo that referenced this pull request Sep 30, 2017
http2stream.respondWithFile api has changed since 8.5.0 with the
addition of the onError option. In the first code example an
onError function is implemented but never used, fix this
mistake.

Add a description to have more informations when onError is triggered.

PR-URL: nodejs/node#15501
Fixes: nodejs/node#15390
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this pull request Oct 3, 2017
http2stream.respondWithFile api has changed since 8.5.0 with the
addition of the onError option. In the first code example an
onError function is implemented but never used, fix this
mistake.

Add a description to have more informations when onError is triggered.

PR-URL: #15501
Fixes: #15390
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins mentioned this pull request Oct 3, 2017
MylesBorins pushed a commit that referenced this pull request Oct 3, 2017
http2stream.respondWithFile api has changed since 8.5.0 with the
addition of the onError option. In the first code example an
onError function is implemented but never used, fix this
mistake.

Add a description to have more informations when onError is triggered.

PR-URL: #15501
Fixes: #15390
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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. http2 Issues or PRs related to the http2 subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

http2: Change in respondWithFile api not properly reflected in the docs

7 participants


Back | FazBrowse Home | New Git URL