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

New: Exit on fatal error (fixes #13711) #14730

Merged
merged 10 commits into from Jul 30, 2021

Conversation

A-Katopodis
Copy link
Contributor

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[x] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

Created a new feature which enables ESLint to exit with code 2 instead of 1 when there are any fatal errors. More on this feature can be found in the RFC.

Is there anything you'd like reviewers to focus on?

The unit test coverage remained the same. The docs where updated as well but the only appropriate change or place to mention the new argument I found was in command-line-interface.md. Let me know if there are places I missed.

@eslint-github-bot eslint-github-bot bot added the triage An ESLint team member will look at this issue soon label Jun 21, 2021
@eslint-github-bot
Copy link

Hi @A-Katopodis!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.

  • The commit message tag must be one of the following:

    The Tag is one of the following:

    • Fix - for a bug fix.
    • Update - either for a backwards-compatible enhancement or for a rule change that adds reported problems.
    • New - implements a new feature.
    • Breaking - for a backwards-incompatible enhancement or feature.
    • Docs - changes to documentation only.
    • Build - changes to build process only.
    • Upgrade - for a dependency upgrade.
    • Chore - for anything that isn't user-facing (for example, refactoring, adding tests, etc.).

    You can use the labels of the issue you are working on to determine the best tag.

  • There should be a space following the initial tag and colon, for example 'New: Message'.

Read more about contributing to ESLint here

@A-Katopodis A-Katopodis changed the title Exit on fatal error New: Exit on fatal error Jun 21, 2021
@mdjermanovic mdjermanovic added accepted There is consensus among the team that this change meets the criteria for inclusion core Relates to ESLint's core APIs and features feature This change adds a new feature to ESLint and removed triage An ESLint team member will look at this issue soon labels Jun 24, 2021
Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

This looks like a good start. I left a few suggestions. I think if you rebase on top of master then you’ll fix the linting errors in CI

docs/user-guide/command-line-interface.md Outdated Show resolved Hide resolved
lib/cli.js Outdated Show resolved Hide resolved
lib/cli.js Outdated Show resolved Hide resolved
@nzakas nzakas linked an issue Jun 26, 2021 that may be closed by this pull request
@nzakas nzakas changed the title New: Exit on fatal error New: Exit on fatal error (fixes #13711) Jun 26, 2021
Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

lib/eslint/eslint.js Outdated Show resolved Hide resolved
docs/user-guide/command-line-interface.md Outdated Show resolved Hide resolved
lib/options.js Outdated Show resolved Hide resolved
tests/lib/cli.js Outdated Show resolved Hide resolved
tests/lib/cli.js Show resolved Hide resolved
@nzakas
Copy link
Member

nzakas commented Jul 1, 2021

@A-Katopodis we'd like to thank you for your work on this PR. Can you shoot me an email at contact (at) eslint (dot) org?

Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

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

Looks good, I left just some small suggestions about documentation and test descriptions.

docs/developer-guide/nodejs-api.md Outdated Show resolved Hide resolved
docs/user-guide/command-line-interface.md Outdated Show resolved Hide resolved
tests/lib/cli.js Outdated Show resolved Hide resolved
tests/lib/cli.js Outdated Show resolved Hide resolved
tests/lib/cli.js Outdated Show resolved Hide resolved
@nzakas
Copy link
Member

nzakas commented Jul 17, 2021

@A-Katopodis can you take a look at the latest review comments?

Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@btmills btmills merged commit 1bfbefd into eslint:master Jul 30, 2021
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Jan 27, 2022
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Jan 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion core Relates to ESLint's core APIs and features feature This change adds a new feature to ESLint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Report fatal parsing errors differently than standard errors
4 participants