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

docs: added documentation for socket.destroyed #6128

Closed
wants to merge 7 commits into from

Conversation

tusharmath
Copy link
Contributor

@tusharmath tusharmath commented Apr 9, 2016

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

doc

Description of change

fixes: #5898

@jasnell jasnell added doc Issues and PRs related to the documentations. net Issues and PRs related to the net subsystem. labels Apr 9, 2016
@tusharmath
Copy link
Contributor Author

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

@@ -359,6 +359,10 @@ The amount of received bytes.

The amount of bytes sent.

### socket.destroyed

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

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For example?

Copy link
Contributor

Choose a reason for hiding this comment

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

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 :).

Copy link
Contributor

Choose a reason for hiding this comment

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

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
Copy link
Contributor

The subsystem should be doc.

@stevemao
Copy link
Contributor

@nodejs/documentation

@benjamingr
Copy link
Member

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

@cjihrig
Copy link
Contributor

cjihrig commented Jun 16, 2016

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

@tusharmath
Copy link
Contributor Author

@cjihrig Made the updates.

@silverwind
Copy link
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
Copy link
Contributor

cjihrig commented Jun 17, 2016

LGTM

@cjihrig
Copy link
Contributor

cjihrig commented Jun 17, 2016

Sorry, I do have one nit:

no data can further

should be

no further data can

@tusharmath
Copy link
Contributor Author

@cjihrig update the changes

@cjihrig
Copy link
Contributor

cjihrig commented Jun 20, 2016

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

@tusharmath
Copy link
Contributor Author

@cjihrig Thanks for pointing it out :)

cjihrig pushed a commit that referenced this pull request Jun 21, 2016
Fixes: #5898
PR-URL: #6128
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@cjihrig
Copy link
Contributor

cjihrig commented Jun 21, 2016

Thanks! Landed in 193afef.

@cjihrig cjihrig closed this Jun 21, 2016
evanlucas pushed a commit that referenced this pull request Jun 27, 2016
Fixes: #5898
PR-URL: #6128
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Fishrock123 pushed a commit that referenced this pull request Jun 27, 2016
Fixes: #5898
PR-URL: #6128
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@Fishrock123 Fishrock123 mentioned this pull request Jun 27, 2016
@Fishrock123 Fishrock123 mentioned this pull request Jul 5, 2016
MylesBorins pushed a commit that referenced this pull request Jul 11, 2016
Fixes: #5898
PR-URL: #6128
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 11, 2016
Fixes: #5898
PR-URL: #6128
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
Fixes: #5898
PR-URL: #6128
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jul 12, 2016
MylesBorins pushed a commit that referenced this pull request Jul 14, 2016
Fixes: #5898
PR-URL: #6128
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 14, 2016
Fixes: #5898
PR-URL: #6128
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
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. net Issues and PRs related to the net subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

socket.destroyed is left undocumented?
8 participants