[WIP] Expose pylint.recommended as default base message set
#10619
Conversation
| # For details: https://github.com/pylint-dev/pylint/blob/main/LICENSE | ||
| # Copyright (c) https://github.com/pylint-dev/pylint/blob/main/CONTRIBUTORS.txt | ||
|
|
||
| core: dict[str, set[str]] = { |
There was a problem hiding this comment.
I don't love the word "core", I was just trying to avoid "all" which could be confusing as it doesn't entail --enable-all-extensions nor the messages "disabled by default" with the checker setting.
There was a problem hiding this comment.
Legacy for 'core' (i.e. pylint prior to pylint 4.0), 'recommended' (current default), extension / all (extension all and enable / all) ? We could add 'google', 'error' ('--error-only'), 'high-confidence' (no inference) and mozzila (although contrary to google I'm not sure there is a conveniant public pylintrc)
| "enable": set(), | ||
| "disable": { | ||
| "duplicate-code", | ||
| # add others from issue... |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Cool idea! Very useful to see a WIP implementation to gather further thoughts for the broader discussion.
If I did a proper full review I'd probably leave some comments about the metavar and _import_attribute. I like _MessageSetsAction!
One big question I have about this implementation is whether we should make the message set a dataclass, object, whatever.
This makes it so that we can do isinstance in the action but more importantly it makes it a little clearer what options are available for these sets and how it should look like. A project trying to create their own message set now has little guidance on how to structure these. If we exposed a simple dataclass that they could instantiate we would avoid this.
Was this something you considered?
82ecd77 to
be64d7d
Compare
Exactly, I figured the datastructure could be polished later. I was racing to get a draft open for discussion :-) |
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (80.00%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #10619 +/- ##
==========================================
- Coverage 95.96% 95.95% -0.02%
==========================================
Files 176 177 +1
Lines 19470 19490 +20
==========================================
+ Hits 18684 18701 +17
- Misses 786 789 +3
🚀 New features to boost your workflow:
|
|
🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉 This comment was generated for commit be64d7d |
| into the given stream or stdout. | ||
| """ | ||
| # If --message-sets is absent, maybe just go ahead and | ||
| # set --message-sets=pylint.recommended? or wait until pylint 5? |
There was a problem hiding this comment.
I trust that what we can devise now is still better than the current default. (But we can wait for 5 too)
| # For details: https://github.com/pylint-dev/pylint/blob/main/LICENSE | ||
| # Copyright (c) https://github.com/pylint-dev/pylint/blob/main/CONTRIBUTORS.txt | ||
|
|
||
| core: dict[str, set[str]] = { |
There was a problem hiding this comment.
Legacy for 'core' (i.e. pylint prior to pylint 4.0), 'recommended' (current default), extension / all (extension all and enable / all) ? We could add 'google', 'error' ('--error-only'), 'high-confidence' (no inference) and mozzila (although contrary to google I'm not sure there is a conveniant public pylintrc)
| # all. Valid levels: HIGH, INFERENCE, INFERENCE_FAILURE, UNDEFINED | ||
| # confidence= | ||
|
|
||
| message-sets=pylint.recommended |
There was a problem hiding this comment.
I don't like the name 'message sets' a lot. Maybe it's because I'd want to put everything inside this (option, message, extension, ... Except things like '--rc-file' or '--version', which will force us to think about it), so I would call it 'style' instead
Type of Changes
Description
Pylint is forbiddingly opinionated and noisy out of the box. For years there has been consensus to "get sane" somehow.
Here's a stab at a design inspired by eslint. Manual testing shows it works.
Provide a list of strings that can be imported from a module on your path. Default is
pylint.recommended. It unpacks to a set of enables and disables that we apply before anything else.Intended to close #3512
We can change the "default" for this setting in 5, once we go through a cycle of verifying that this works and can be customized (e.g.
pylint_django.recommendedor whatever)