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
Merged

Conversation

@A-Katopodis
Copy link
Contributor

@A-Katopodis A-Katopodis commented Jun 21, 2021

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

@eslint-github-bot eslint-github-bot bot commented Jun 21, 2021

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

@nzakas nzakas left a comment

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 that may be closed by this pull request Jun 26, 2021
@nzakas nzakas changed the title New: Exit on fatal error New: Exit on fatal error (fixes #13711) Jun 26, 2021
nzakas
nzakas approved these changes Jun 29, 2021
Copy link
Member

@nzakas nzakas left a comment

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 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

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 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

LGTM, thanks!

@btmills btmills merged commit 1bfbefd into eslint:master Jul 30, 2021
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants