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

Updated reference to signal list in "signal events" section #9323

Closed
wants to merge 3 commits into from

Conversation

emadb
Copy link
Contributor

@emadb emadb commented Oct 27, 2016

Documentation is changed or added.
Changed the reference from sigaction(2) to signal(7) like requested in issue #9309

Changed the reference from sigaction(2) to signal(7) like requested in issue nodejs#9309
@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. process Issues and PRs related to the process subsystem. labels Oct 27, 2016
@@ -374,7 +374,7 @@ The `*-deprecation` command line flags only affect warnings that use the name
<!--name=SIGINT, SIGHUP, etc.-->

Signal events will be emitted when the Node.js process receives a signal. Please
refer to sigaction(2) for a listing of standard POSIX signal names such as
refer to [signal(7)](http://man7.org/linux/man-pages/man7/signal.7.html) for a listing of standard POSIX signal names such as
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you move the URL to the bottom of the page with the others.

@mscdex
Copy link
Contributor

mscdex commented Oct 27, 2016

I don't think a link needs to be explicitly added since the doc tools automatically insert links when they see man page references. So just changing the 'sigaction(2)' to 'signal(7)' should be enough.

@emadb
Copy link
Contributor Author

emadb commented Oct 27, 2016

@mscdex didn't about it. I wait for a feedback from @cjihrig if I have to remove the link.
Thx (this is my very first try to help).

@cjihrig
Copy link
Contributor

cjihrig commented Oct 27, 2016

Sorry @emadb, I didn't remember that the doc tools could recognize man pages. @mscdex is right.

I removed the link since the doc tool automatically add link to man pages.
@mscdex
Copy link
Contributor

mscdex commented Oct 27, 2016

LGTM

@sam-github
Copy link
Contributor

@cjihrig I think you have to mark your requested changes as having been made (I'm not loving the new review system yet).

LGTM

sam-github pushed a commit that referenced this pull request Oct 27, 2016
Fixes: #9309
PR-URL: #9323
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Brian White <[email protected]>
@sam-github
Copy link
Contributor

Landed in 6893f3c, thanks @emadb

@sam-github sam-github closed this Oct 27, 2016
evanlucas pushed a commit that referenced this pull request Nov 3, 2016
Fixes: #9309
PR-URL: #9323
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Brian White <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 11, 2016
Fixes: #9309
PR-URL: #9323
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Brian White <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 11, 2016
Fixes: #9309
PR-URL: #9323
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Brian White <[email protected]>
This was referenced Nov 22, 2016
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. process Issues and PRs related to the process subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants