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

Exit immediately if all messages are disabled #8799

Merged
merged 2 commits into from
Jun 26, 2023

Conversation

jacobtylerwalls
Copy link
Member

@jacobtylerwalls jacobtylerwalls commented Jun 26, 2023

Type of Changes

Type
βœ“ πŸ”¨ Refactoring

Description

Closes #8715 (should reduce the number of issues we get about this)

Exit immediately with the help message if all checks are disabled. Exit non-zero.

def test_disable_all(self) -> None:
out = StringIO()
self._runtest([UNNECESSARY_LAMBDA, "--disable=all"], out=out, code=0)
assert "show this help message and exit" in out.getvalue().strip()
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm torn on whether to print help in this case. A user might have unintentionally disabled everything due to the order of command-line arguments.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, what about a simple No files to lint, exiting ? The help message a is a wall of text and it's not intuitive that there wasn't any files to lint when getting a wall of text..

@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

πŸ€– According to the primer, this change has no effect on the checked open source code. πŸ€–πŸŽ‰

This comment was generated for commit c410636

Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

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

That's great, because often it's the first test that someone is doing when opening an issue saying that pylint is too slow. It's not going to make pylint faster in real world use case or magically create @only_required_for for disabled message but it's going to make pylint succeed at this sanity check while also displaying a nice warning message for those who disable everything πŸ‘Œ

@jacobtylerwalls jacobtylerwalls merged commit d203de7 into pylint-dev:main Jun 26, 2023
@jacobtylerwalls jacobtylerwalls deleted the disable-all branch June 26, 2023 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Short circuit if all checks disabled
3 participants