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.

Sign up for GitHub

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

Jump to bottom

doc: add extends for derived classes #29290

Closed
wants to merge 3 commits into from
Closed

doc: add extends for derived classes #29290

wants to merge 3 commits into from

Conversation

trivikr
Copy link
Member

@trivikr trivikr commented Aug 24, 2019

Checklist

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Aug 24, 2019
@trivikr trivikr changed the title Doc extends doc: add extends for derived classes Aug 24, 2019
@mscdex
Copy link
Contributor

mscdex commented Aug 24, 2019

I think the classes being extended should still be linked for easier reference.

@trivikr
Copy link
Member Author

trivikr commented Aug 24, 2019

We're using Extends {ClassName} at many other places in the documentation (http, http2, worker threads etc) and seems self-explanatory. Description of classes being extended seems redundant.

@trivikr trivikr added child_process Issues and PRs related to the child_process subsystem. cluster Issues and PRs related to the cluster subsystem. dgram Issues and PRs related to the dgram subsystem / UDP. domain Issues and PRs related to the domain subsystem. inspector Issues and PRs related to the V8 inspector protocol net Issues and PRs related to the net subsystem. readline Issues and PRs related to the built-in readline module. repl Issues and PRs related to the REPL subsystem. tty Issues and PRs related to the tty subsystem. labels Aug 24, 2019
@mscdex
Copy link
Contributor

mscdex commented Aug 24, 2019 •
edited
Loading

Description of classes being extended seems redundant

Why? Not all inherited methods/properties/etc. are shown in docs for classes that are extending other classes. Being able to click on the parent class name makes it easy to see what is inherited.

@trivikr
Copy link
Member Author

trivikr commented Aug 24, 2019

The classname after Extends is clickable :-)

@trivikr trivikr mentioned this pull request Aug 29, 2019
Open
@trivikr trivikr added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 29, 2019
@trivikr
Copy link
Member Author

trivikr commented Sep 1, 2019

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

@trivikr
Copy link
Member Author

trivikr commented Sep 1, 2019

Landed in 89aea15

@trivikr trivikr closed this Sep 1, 2019
@trivikr trivikr deleted the doc-extends branch September 1, 2019 15:19
BridgeAR pushed a commit that referenced this pull request Sep 3, 2019
@trivikr @BridgeAR
doc: add extends for derived classes
71bc32c 
PR-URL: #29290
Reviewed-By: James M Snell <jasnell@gmail.com>
@BridgeAR BridgeAR mentioned this pull request Sep 3, 2019
Merged
BridgeAR pushed a commit that referenced this pull request Sep 4, 2019
@trivikr @BridgeAR
doc: add extends for derived classes
5a16449 
PR-URL: #29290
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Fishrock123 Fishrock123 Fishrock123 left review comments

@Trott Trott Trott left review comments

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. child_process Issues and PRs related to the child_process subsystem. cluster Issues and PRs related to the cluster subsystem. dgram Issues and PRs related to the dgram subsystem / UDP. doc Issues and PRs related to the documentations. domain Issues and PRs related to the domain subsystem. inspector Issues and PRs related to the V8 inspector protocol net Issues and PRs related to the net subsystem. readline Issues and PRs related to the built-in readline module. repl Issues and PRs related to the REPL subsystem. tty Issues and PRs related to the tty subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@trivikr @mscdex @jasnell @Trott @Fishrock123 @nodejs-github-bot