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

docs: added documentation for socket.destroyed by tusharmath · Pull Request #6128 · nodejs/node · GitHub

/ node Public

docs: added documentation for socket.destroyed - #6128

Closed
tusharmath wants to merge 7 commits into
nodejs:masterfrom
tusharmath:patch-1
ConversationCommits (7)ChecksFiles changed
Closed

docs: added documentation for socket.destroyed#6128
tusharmath wants to merge 7 commits into
nodejs:masterfrom
tusharmath:patch-1

Conversation

tusharmath commented Apr 9, 2016 •
edited
Loading

Copy link
Copy Markdown
Contributor
Checklist
  • documentation is changed or added
  • the commit message follows commit guidelines
Affected core subsystem(s)

doc

Description of change

fixes: #5898

jasnell added doc Issues and PRs related to the documentations. net Issues and PRs related to the net subsystem. labels Apr 9, 2016

tusharmath commented Apr 9, 2016

Copy link
Copy Markdown
Contributor Author

My apologies if some guidelines haven't been met. This is my first contribution.

silverwind reviewed Apr 9, 2016
View reviewed changes
Comment thread doc/api/net.markdown Outdated

### socket.destroyed

A `boolean` value that depicts if the connection is destroyed or not.

silverwind Apr 9, 2016

Copy link
Copy Markdown
Contributor

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

Can we add a bit more detail about what "destroyed" means?

tusharmath Apr 9, 2016

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

For example?

silverwind Apr 9, 2016

Copy link
Copy Markdown
Contributor

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

Look into the source to find out, I think something along the lines of "does not send or receive any more data, cannot be re-established", but I'm no expert on this topic :).

svozza Apr 19, 2016

Copy link
Copy Markdown
Contributor

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

I think "indicates" might be a better choice of word than "depicts". Maybe something like this:

A Boolean value that indicates whether the connection has been destroyed or not.

silverwind commented Apr 9, 2016

Copy link
Copy Markdown
Contributor

The subsystem should be doc.

stevemao commented Apr 21, 2016

Copy link
Copy Markdown
Contributor

@nodejs/documentation

benjamingr commented Apr 24, 2016

Copy link
Copy Markdown
Member

Hey, thanks, please wrap lines at 80 chars :)

estliberitas force-pushed the master branch 2 times, most recently from 7da4fd4 to c7066fb Compare April 26, 2016 05:23

cjihrig commented Jun 16, 2016

Copy link
Copy Markdown
Contributor

@tusharmath ping. Are you still planning to work on this?

tusharmath commented Jun 16, 2016

Copy link
Copy Markdown
Contributor Author

@cjihrig Made the updates.

silverwind commented Jun 16, 2016

Copy link
Copy Markdown
Contributor

Might be me, but I think this sounds better:

A boolean value that indicates if the connection is destroyed, meaning it will not accept or deliver any more data.

Not strictly necessary for this PR, but I think some information whether a half-closed socket can be in a destroyed state might be good to add as well (haven't researched).

cjihrig commented Jun 17, 2016

Copy link
Copy Markdown
Contributor

LGTM

cjihrig commented Jun 17, 2016

Copy link
Copy Markdown
Contributor

Sorry, I do have one nit:

no data can further

should be

no further data can

tusharmath commented Jun 20, 2016

Copy link
Copy Markdown
Contributor Author

@cjihrig update the changes

cjihrig commented Jun 20, 2016

Copy link
Copy Markdown
Contributor

Sorry, I just realized this - can you maintain the alphabetical order of the docs.

tusharmath commented Jun 20, 2016

Copy link
Copy Markdown
Contributor Author

@cjihrig Thanks for pointing it out :)

cjihrig pushed a commit that referenced this pull request Jun 21, 2016
doc: document socket.destroyed
193afef 
Fixes: #5898
PR-URL: #6128
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

cjihrig commented Jun 21, 2016

Copy link
Copy Markdown
Contributor

Thanks! Landed in 193afef.

cjihrig closed this Jun 21, 2016
cjihrig mentioned this pull request Jun 22, 2016
Closed
5 tasks
evanlucas pushed a commit that referenced this pull request Jun 27, 2016
doc: document socket.destroyed
dddfed2 
Fixes: #5898
PR-URL: #6128
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Fishrock123 pushed a commit that referenced this pull request Jun 27, 2016
doc: document socket.destroyed
5c6e24e 
Fixes: #5898
PR-URL: #6128
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Fishrock123 mentioned this pull request Jun 27, 2016
Merged
Fishrock123 mentioned this pull request Jul 5, 2016
Merged
MylesBorins pushed a commit that referenced this pull request Jul 11, 2016
doc: document socket.destroyed
4d6e405 
Fixes: #5898
PR-URL: #6128
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 11, 2016
doc: document socket.destroyed
6b4b6cb 
Fixes: #5898
PR-URL: #6128
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
doc: document socket.destroyed
159d47c 
Fixes: #5898
PR-URL: #6128
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins mentioned this pull request Jul 12, 2016
Merged
MylesBorins pushed a commit that referenced this pull request Jul 14, 2016
doc: document socket.destroyed
a6113bc 
Fixes: #5898
PR-URL: #6128
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 14, 2016
doc: document socket.destroyed
5380743 
Fixes: #5898
PR-URL: #6128
Reviewed-By: Colin Ihrig <cjihrig@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

No reviews

Assignees

No one assigned

Labels

doc Issues and PRs related to the documentations. net Issues and PRs related to the net subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

socket.destroyed is left undocumented?

8 participants

Back | FazBrowse Home | New Git URL