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

doc: specify `options` parameter type in zlib.md by vsemozhetbyt · Pull Request #21920 · nodejs/node · GitHub

/ node Public

doc: specify options parameter type in zlib.md - #21920

Closed
vsemozhetbyt wants to merge 1 commit into
nodejs:masterfrom
vsemozhetbyt:doc-zlib-options-type
ConversationCommits (1)ChecksFiles changed
Closed

doc: specify options parameter type in zlib.md#21920
vsemozhetbyt wants to merge 1 commit into
nodejs:masterfrom
vsemozhetbyt:doc-zlib-options-type

Conversation

vsemozhetbyt commented Jul 21, 2018

Copy link
Copy Markdown
Contributor
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

Also, delete now redundant notes and links.

nodejs-github-bot commented Jul 21, 2018

Copy link
Copy Markdown
Collaborator

@vsemozhetbyt build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/326/pipeline

vsemozhetbyt added doc Issues and PRs related to the documentations. zlib Issues and PRs related to the zlib subsystem. tools Issues and PRs related to the tools directory. labels Jul 21, 2018

vsemozhetbyt commented Jul 21, 2018

Copy link
Copy Markdown
Contributor Author

Node.js Collaborators, please, add 👍 here if you approve fast-tracking.

lpinca reviewed Jul 21, 2018
View reviewed changes
Comment thread doc/api/zlib.md Outdated

lpinca Jul 21, 2018

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

Hmm, this makes me think that zlib.Options is a constructor I have to instantiate instead of a plain object. I have to click the link to understand that in reality is a plain object.

vsemozhetbyt Jul 21, 2018

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

Commit amended. Would it be better in this way?

lpinca Jul 21, 2018

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

Yes, I think it's better. Thanks.

vsemozhetbyt Jul 21, 2018

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

We have something similar for http2:

https://github.com/vsemozhetbyt/node/blob/7938680ebb3e1b3319e274c864d71b0b593a862c/tools/doc/type-parser.js#L74-L75

doc: specify options parameter type in zlib.md
7938680 
Also, delete now redundant notes and links.

vsemozhetbyt commented Jul 21, 2018

Copy link
Copy Markdown
Contributor Author

CI-lite: https://ci.nodejs.org/job/node-test-pull-request-lite/925/

vsemozhetbyt commented Jul 24, 2018

Copy link
Copy Markdown
Contributor Author

Landed in 811598b
Thank you for the reviews.

vsemozhetbyt deleted the doc-zlib-options-type branch July 24, 2018 14:02
vsemozhetbyt added a commit that referenced this pull request Jul 24, 2018
doc: specify options parameter type in zlib.md
811598b 
Also, delete now redundant notes and links.

PR-URL: #21920
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

targos commented Jul 24, 2018

Copy link
Copy Markdown
Member

Depends on #21782 to land on v10.x-staging

targos pushed a commit that referenced this pull request Aug 7, 2018
doc: specify options parameter type in zlib.md
e80c646 
Also, delete now redundant notes and links.

PR-URL: #21920
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
rvagg mentioned this pull request Aug 13, 2018
Merged
This was referenced Aug 16, 2018
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
firass111 pushed a commit to firass111/Project_node1 that referenced this pull request Apr 16, 2025
doc: specify options parameter type in zlib.md
e75885f 
Also, delete now redundant notes and links.

PR-URL: nodejs/node#21920
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

lpinca lpinca approved these changes
trivikr trivikr approved these changes

Assignees

No one assigned

Labels

doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory. zlib Issues and PRs related to the zlib subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL