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

pylint equivalent to pylint . #9072

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Pierre-Sassoulas
Copy link
Member

Type of Changes

Type
✨ New feature

Description

pylint is now equivalent to pylint . and won't show the help by default anymore.
The help is still available with pylint --help or pylint --long-help.

Closes #5701

@codecov
Copy link

codecov bot commented Sep 25, 2023

Codecov Report

Merging #9072 (b2b0571) into main (31f348f) will increase coverage by 0.02%.
Report is 1 commits behind head on main.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #9072      +/-   ##
==========================================
+ Coverage   95.75%   95.77%   +0.02%     
==========================================
  Files         173      173              
  Lines       18664    18669       +5     
==========================================
+ Hits        17872    17881       +9     
+ Misses        792      788       -4     
Files Coverage Δ
pylint/config/arguments_manager.py 99.45% <ø> (-0.01%) ⬇️
pylint/lint/pylinter.py 96.66% <100.00%> (+0.43%) ⬆️
pylint/lint/run.py 87.50% <100.00%> (ø)

... and 1 file with indirect coverage changes

@Pierre-Sassoulas
Copy link
Member Author

Pierre-Sassoulas commented Sep 25, 2023

Hmm, we might want to have it in a beta for a somewhat longish time, this feel incompatible with releasing in a week. On one hand the change is not supposed to be complex, on the other hand it actually is complex. Because the option parsing and configuration parsing is very complex (there's no explicit positional argument like you would expect with argparse, we're just implicitly relying on the first args being the fileçor_module if we don't have the stdin option -- maybe other condition apply).

@github-actions

This comment has been minimized.

Copy link
Member

@jacobtylerwalls jacobtylerwalls left a comment

Choose a reason for hiding this comment

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

Thanks!

Wondering if you checked pyreverse?

pylint/lint/pylinter.py Show resolved Hide resolved
@@ -176,10 +176,8 @@ def __init__(
return

# Display help if there are no files to lint or no checks enabled
Copy link
Member

Choose a reason for hiding this comment

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

May as well clean up this comment, as we're not displaying help (right?)

self._runtest(
["--ignore-patterns=a"], reporter=TextReporter(StringIO()), code=32
Copy link
Member

Choose a reason for hiding this comment

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

What was making the prior code emit 32?

pylint/config/arguments_manager.py Show resolved Hide resolved
Comment on lines +1 to +2
``pylint`` is now equivalent to ``pylint .`` and won't show the help by default anymore.
The help is still available with ``pylint --help`` or ``pylint --long-help``.
Copy link
Member

Choose a reason for hiding this comment

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

Let's not promise equivalence with pylint .. I'm showing a behavior difference. Before, cd to a directory you haven't installed with pip, invoke pylint ., you get a quick lint of the __init__.py only; now you get the whole package linted.

@Pierre-Sassoulas
Copy link
Member Author

I'm thinking more and more that we should base it on https://github.com/pylint-dev/pylint/pull/7496/files so we can just add a default value for files instead of hacking the implicit list of args.

@DanielNoord
Copy link
Collaborator

I'm thinking more and more that we should base it on https://github.com/pylint-dev/pylint/pull/7496/files so we can just add a default value for files instead of hacking the implicit list of args.

I would be very much in favour of that, but need some direction in moving that PR forward.

@Pierre-Sassoulas
Copy link
Member Author

Let's move that change to 4.0.0 and try to add the files option in 3.1.0, I don't want to rush this.

```
    def parse_known_args(self, args=None, namespace=None):
        if args is None:
            # args default to the system args
            args = _sys.argv[1:]
        else:
            # make sure that args are mutable
            args = list(args)
```
@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 b2b0571

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.

Should pylint be equivalent to pylint . ?
3 participants