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: remove old errors_system_errors #26976

Closed
wants to merge 1 commit into from
Closed

doc: remove old errors_system_errors #26976

wants to merge 1 commit into from

Conversation

JungMinu
Copy link
Member

@JungMinu JungMinu commented Mar 29, 2019 •
edited
Loading

  • remove old errors_system_errors which partially predate the existence of the class, any useful information in it moved into the SystemError class docs
  • outdent SystemError class docs to the same level as all the other classes
Checklist

@JungMinu
doc: remove old system_errors
47c255b 
remove old errors_system_errors, any useful information
in it moved into the SystemError class docs
@nodejs-github-bot
Copy link
Collaborator

@JungMinu Sadly, an error occurred when I tried to trigger a build. :(

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Mar 29, 2019
@vsemozhetbyt vsemozhetbyt added the errors Issues and PRs related to JavaScript errors originated in Node.js core. label Mar 29, 2019
BridgeAR
BridgeAR reviewed Mar 29, 2019
View reviewed changes
doc/api/errors.md
attempts to read a file that does not exist.

System errors are usually generated at the syscall level. For a comprehensive
list, see the [`errno`(3) man page][].
Copy link
Member

Choose a reason for hiding this comment

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

Should we really remove the whole description? It still seems partially useful to me?

Copy link
Member Author

Choose a reason for hiding this comment

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

See #26861 for context

Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't see Ruben's comment because it was hidden/resolved. I agree with him.

sam-github
sam-github reviewed Mar 29, 2019
View reviewed changes
doc/api/errors.md
@@ -442,19 +439,7 @@ Some exceptions are *unrecoverable* at the JavaScript layer. Such exceptions
will *always* cause the Node.js process to crash. Examples include `assert()`
checks or `abort()` calls in the C++ layer.

## System Errors

Node.js generates system errors when exceptions occur within its runtime
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 these two paragraphs, slightly edited, should be moved to be the description of class SystemError, because the information in them is still useful.

@sam-github
Copy link
Contributor

This is intended to fix #26861, I believe. Could you edit the commit description to change it from:

remove old errors_system_errors, any useful information
in it moved into the SystemError class docs

to be punctuated and capitalized:

Remove ..
.... class docs.

Fixes: https://github.com/nodejs/node/issues/26861

@JungMinu JungMinu closed this Mar 29, 2019
@sam-github
Copy link
Contributor

@JungMinu why did you close this? If it was to change the commit message, you can do that with git commit --amend, and then do a git push --force-with-lease, and the PR will be updated in place.

@sam-github sam-github reopened this Mar 30, 2019
@JungMinu
Copy link
Member Author

JungMinu commented Apr 1, 2019

@sam-github I know that but was already in the middle of updating it in another PR, closing this PR

@JungMinu JungMinu closed this Apr 1, 2019
@JungMinu JungMinu deleted the fix-26861 branch April 1, 2019 01:53
@baby636 baby636 mentioned this pull request Jul 12, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BridgeAR BridgeAR BridgeAR left review comments

@sam-github sam-github sam-github left review comments

Assignees
No one assigned
Labels
doc Issues and PRs related to the documentations. errors Issues and PRs related to JavaScript errors originated in Node.js core.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@JungMinu @nodejs-github-bot @sam-github @BridgeAR @vsemozhetbyt