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

Deprecate extension-pkg-whitelist #7118

Closed
wants to merge 2 commits into from

Conversation

DanielNoord
Copy link
Collaborator

  • Add yourself to CONTRIBUTORS if you are a new contributor.
  • Write a good description on what the PR does.

Type of Changes

Type
🔨 Refactoring

Description

Ref. #5465

Doesn't close as I also need to deprecate the black_list options.

@DanielNoord DanielNoord added the Maintenance Discussion or action around maintaining pylint or the dev workflow label Jul 3, 2022
@coveralls
Copy link

coveralls commented Jul 3, 2022

Pull Request Test Coverage Report for Build 2605822650

  • 9 of 9 (100.0%) changed or added relevant lines in 3 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.005%) to 95.353%

Files with Coverage Reduction New Missed Lines %
pylint/utils/utils.py 1 86.36%
Totals Coverage Status
Change from base Build 2601041076: -0.005%
Covered Lines: 16703
Relevant Lines: 17517

💛 - Coveralls

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.

I don't think we should actually deprecate this one. If we have the capability to auto-upgrade the configuration we don't need to deprecate, and if we don't then it's forcing work on our user to fix something that is not broken.

@DanielNoord
Copy link
Collaborator Author

I mean, I'm not personally offended by this in anyway but if some users are, why wouldn't we put in a little effort to alleviate that? We already have the functionality to do so, so why not use it? I get that the discussion is a bit political, but it doesn't really hurt those who aren't offended and might help those who are.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2022

🤖 Effect of this PR on checked open source code: 🤖

Effect on black:
The following messages are no longer emitted:

  1. attribute-defined-outside-init:
    Attribute 'symbol2number' defined outside init
    https://github.com/psf/black/blob/main/src/blib2to3/pgen2/conv.py#L68
  2. attribute-defined-outside-init:
    Attribute 'number2symbol' defined outside init
    https://github.com/psf/black/blob/main/src/blib2to3/pgen2/conv.py#L69
  3. attribute-defined-outside-init:
    Attribute 'states' defined outside init
    https://github.com/psf/black/blob/main/src/blib2to3/pgen2/conv.py#L164
  4. attribute-defined-outside-init:
    Attribute 'dfas' defined outside init
    https://github.com/psf/black/blob/main/src/blib2to3/pgen2/conv.py#L195
  5. attribute-defined-outside-init:
    Attribute 'labels' defined outside init
    https://github.com/psf/black/blob/main/src/blib2to3/pgen2/conv.py#L216
  6. attribute-defined-outside-init:
    Attribute 'start' defined outside init
    https://github.com/psf/black/blob/main/src/blib2to3/pgen2/conv.py#L238
  7. attribute-defined-outside-init:
    Attribute 'keywords' defined outside init
    https://github.com/psf/black/blob/main/src/blib2to3/pgen2/conv.py#L250
  8. attribute-defined-outside-init:
    Attribute 'tokens' defined outside init
    https://github.com/psf/black/blob/main/src/blib2to3/pgen2/conv.py#L251

Effect on django:
The following messages are now emitted:

  1. no-member:
    Method 'new' has no 'members' member
    https://github.com/django/django/blob/main/django/db/models/enums.py#L30
  2. no-member:
    Method 'new' has no 'browser' member
    https://github.com/django/django/blob/main/django/test/selenium.py#L30
  3. no-member:
    Method 'new' has no 'browsers' member
    https://github.com/django/django/blob/main/django/test/selenium.py#L34
  4. no-member:
    Method 'new' has no 'browsers' member
    https://github.com/django/django/blob/main/django/test/selenium.py#L39
  5. no-member:
    Method 'new' has no 'selenium_hub' member
    https://github.com/django/django/blob/main/django/test/selenium.py#L42
  6. no-member:
    Method 'new' has no 'host' member
    https://github.com/django/django/blob/main/django/test/selenium.py#L42
  7. no-member:
    Method 'new' has no 'browsers' member
    https://github.com/django/django/blob/main/django/test/selenium.py#L48
  8. no-value-for-parameter:
    No value for argument 'value' in classmethod call
    https://github.com/django/django/blob/main/django/utils/deconstruct.py#L17
  9. no-member:
    Method 'new' has no 'declared_fields' member
    https://github.com/django/django/blob/main/django/forms/models.py#L332
  10. no-member:
    Method 'new' has no 'mro' member
    https://github.com/django/django/blob/main/django/forms/forms.py#L40

The following messages are no longer emitted:

  1. invalid-name:
    Attribute name "Meta" doesn't conform to '[a-z_][a-z0-9_]{2,}$' pattern
    https://github.com/django/django/blob/main/django/db/models/base.py#L360

This comment was generated for commit 4d06e5c

@Pierre-Sassoulas
Copy link
Member

but if some users are, why wouldn't we put in a little effort to alleviate that?

It's not only us doing the effort but every users that will need to upgrade their conf for 3.0. I don't want users to have to upgrade their conf in general... but in particular I don't want them to have to upgrade their configuration for political reasons. And I especially don't want to deal with bigots that had to upgrade their configuration and now have a strong desire to tell us about the etymology of blacklist. Let's not create political breaking change, and let's not trigger anyone.

If we do #5462 then we'll be able to clean up configuration. The amount of work to do this issue is tiny in comparison of the work involved for all pylint users to upgrade all their conf and probably comparable to the bigot handling we'd have to do for this anyway.

@DanielNoord
Copy link
Collaborator Author

Should we close the related issue then?

@jacobtylerwalls I noticed the primer diff here. Something is up with my recent astroid PR 😅

@Pierre-Sassoulas
Copy link
Member

Yeah, sorry about that, #5465 only has reference to a very lengthy discussion in another PR so it's not self evident what needed to be done. I upgraded the issue as blocked by #5462

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Maintenance Discussion or action around maintaining pylint or the dev workflow
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants