Skip to content

Conversation

@DanielNoord
Copy link
Collaborator

  • Add yourself to CONTRIBUTORS if you are a new contributor.
  • Add a ChangeLog entry describing what your PR does.
  • If it's a new feature, or an important bug fix, add a What's New entry in
    doc/whatsnew/<current release.rst>.
  • Write a good description on what the PR does.

Type of Changes

Type
🐛 Bug fix

Description

Crash identified while working on #5547

Tagging @mbyrnepr2 since you contributed this checker and I wonder if you have an opinion on this fix.

@DanielNoord DanielNoord added the Crash 💥 A bug that makes pylint crash label Dec 17, 2021
@DanielNoord DanielNoord added this to the 2.13.0 milestone Dec 17, 2021
@coveralls
Copy link

coveralls commented Dec 17, 2021

Pull Request Test Coverage Report for Build 1594218126

  • 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.007%) to 93.667%

Totals Coverage Status
Change from base Build 1593819838: 0.007%
Covered Lines: 14273
Relevant Lines: 15238

💛 - 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.

👍

Comment on lines 882 to 883
else:
continue
Copy link
Member

Choose a reason for hiding this comment

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

It look like this is never used, maybe we can remove it ? Beside right now nothing is exactly the same behavior, right ?

Suggested change
else:
continue

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is used on 3.6 and 3.7. Coveralls is reported on 3.8 (I believe) that's why the coverage dropped. I have added a pragma: no cover.

@mbyrnepr2
Copy link
Member

Thank you for the headsup @DanielNoord much appreciated and thanks for catching it & fixing.
The suggestions in the code come to mind but I trust your judgement 👍

@Pierre-Sassoulas Pierre-Sassoulas merged commit ddd2123 into pylint-dev:main Dec 17, 2021
@DanielNoord DanielNoord deleted the final-crash-checker branch December 17, 2021 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Crash 💥 A bug that makes pylint crash

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants