Skip to content

Move HUMAN_READABLE_TYPES to constants #5642

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

Merged
merged 1 commit into from
Jan 5, 2022

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

I'm going to go through the effort of picking some of the changes for #5311 into separate PRs.
I don't see myself being able to keep my focus for 2000+ lines 😄

I'm co-authoring @CarliJoy in all of them as they have done all the work here!

@DanielNoord DanielNoord added the Maintenance Discussion or action around maintaining pylint or the dev workflow label Jan 5, 2022
@DanielNoord DanielNoord added this to the 2.13.0 milestone Jan 5, 2022
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.

There will be a lot to rebase, but this will surely help. I don't know if it's a reasonable use of our time though. The main issue with the other MR is also that a lot of new lines of code were added in two new checkers at once.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 1660689174

  • 4 of 4 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 93.701%

Totals Coverage Status
Change from base Build 1658866144: 0.0%
Covered Lines: 14354
Relevant Lines: 15319

💛 - Coveralls

@DanielNoord
Copy link
Collaborator Author

I'm working on splitting out the move of non-ascii-name next, that can definetly be done in a separate PR. I'll see what I can do afterwards. Doing this PRs also helps with actually checking the changes.

@DanielNoord DanielNoord merged commit 29997f8 into pylint-dev:main Jan 5, 2022
@DanielNoord DanielNoord deleted the human-read branch January 5, 2022 23:03
@Pierre-Sassoulas
Copy link
Member

Yes we can definitely do a MR for each new checker.

@DanielNoord
Copy link
Collaborator Author

Little bit more annoying than I thought since you can't change the "owner" of a PR. So I can't easily ask questions about things to @CarliJoy. Guess I'll keep working in that PR then.

@Pierre-Sassoulas
Copy link
Member

If I were you I would let it go and review one Checker after the other now that the two files are separated. It sure is a pain to review but so is splitting it up then still have to review 😄

areveny pushed a commit to areveny/pylint that referenced this pull request Jan 9, 2022
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