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

[too-many-positional-arguments] Better documentation following questions #10049

Merged
merged 2 commits into from
Oct 29, 2024

Conversation

Pierre-Sassoulas
Copy link
Member

Type of Changes

Type
βœ“ πŸ“œ Docs

Description

Add clearer explanation for those who don't know about special parameters, cleanup remnant of the time the check was not implemented in pylint and only reserved the msgid/symbol to mirror ruff.

See https://github.com/astral-sh/ruff/issues/8946\#issuecomment-2437547742

@Pierre-Sassoulas Pierre-Sassoulas added Documentation πŸ“— Skip news πŸ”‡ This change does not require a changelog entry labels Oct 28, 2024
@Pierre-Sassoulas
Copy link
Member Author

@flying-sheep I credited you as co-author in the commit if you don't mind.

Copy link

codecov bot commented Oct 28, 2024

Codecov Report

All modified and coverable lines are covered by tests βœ…

Project coverage is 95.80%. Comparing base (71e2d0f) to head (8e238ba).
Report is 59 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main   #10049   +/-   ##
=======================================
  Coverage   95.80%   95.80%           
=======================================
  Files         174      174           
  Lines       18937    18937           
=======================================
  Hits        18143    18143           
  Misses        794      794           

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.

Great! Left some minor suggestions.

Co-authored-by: Jacob Walls <jacobtylerwalls@gmail.com>
@Pierre-Sassoulas
Copy link
Member Author

Thank you ! Minor suggestions, major improvement πŸ‘Œ I tried to reuse the ``----- Keyword```from the python doc, then black destroyed everything by autoformatting, then I forgot to document in proper English πŸ˜„

@Pierre-Sassoulas Pierre-Sassoulas enabled auto-merge (squash) October 29, 2024 08:01
@mbyrnepr2 mbyrnepr2 enabled auto-merge (squash) October 29, 2024 09:30
Copy link
Member

@mbyrnepr2 mbyrnepr2 left a comment

Choose a reason for hiding this comment

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

Perhaps for another day - but currently we raise this message for positional-or-keyword arguments which is potentially problematic since we don't know if the function will be called with keywords or not.

@mbyrnepr2 mbyrnepr2 merged commit 4ddd43e into pylint-dev:main Oct 29, 2024
26 checks passed
@Pierre-Sassoulas Pierre-Sassoulas deleted the doc-too-many-positionals branch October 29, 2024 10:18
@jacobtylerwalls
Copy link
Member

Perhaps for another day - but currently we raise this message for positional-or-keyword arguments which is potentially problematic since we don't know if the function will be called with keywords or not.

I guess I don't see that as a problem. I think the point of this message is that since you don't know whether it will be called with keywords or not, you should force extra arguments to be called by keyword.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation πŸ“— Skip news πŸ”‡ This change does not require a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants