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

[primer] Create a class for easier comparison of pylint results #6984

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Pierre-Sassoulas
Copy link
Member

Type of Changes

Type
βœ“ πŸ”¨ Refactoring

Description

Follow up to #6973

@Pierre-Sassoulas Pierre-Sassoulas added the Maintenance Discussion or action around maintaining pylint or the dev workflow label Jun 20, 2022
@coveralls
Copy link

coveralls commented Jun 20, 2022

Pull Request Test Coverage Report for Build 2528939077

  • 37 of 37 (100.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.002%) to 95.287%

Totals Coverage Status
Change from base Build 2527590849: 0.002%
Covered Lines: 16600
Relevant Lines: 17421

πŸ’› - Coveralls

@github-actions
Copy link
Contributor

πŸ€– According to the primer, this change has no effect on the checked open source code. πŸ€–πŸŽ‰

This comment was generated for commit a54c77e

pylint/testutils/_primer/comparator.py Show resolved Hide resolved
pylint/testutils/_primer/primer_command.py Outdated Show resolved Hide resolved
from pylint.testutils._primer.primer_command import Messages, PackageMessages


class Comparator:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you rename this to PylintComparator? At some point I want to make an AstroidComparator πŸ˜„

If you're up for it perhaps we could even create an abstract class?

Copy link
Member Author

@Pierre-Sassoulas Pierre-Sassoulas Jun 22, 2022

Choose a reason for hiding this comment

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

What do you mean by astroid comparator ? Comparing a json dump of the generated ast ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, using the primer on astroid/main to catch crashes there before we merge them. That would require a different Comparator I think.

Copy link
Member Author

@Pierre-Sassoulas Pierre-Sassoulas Jun 22, 2022

Choose a reason for hiding this comment

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

So it's the basic primer for pylint but for astroid ? It does not need a comparator right ? As if a crash exists it's always problem, and there's no crash on main.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, but I thinking of maybe running the pylint test suite on some version on astroid main or something like that. The overall goal is to remove the need for pylint-tested label and have all PRs get that automatically.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah sure I understood that :) It make sense to test astroid more in the astroid CI, because pylint's CI is already crowded so I think it's a really good idea too.

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 primer Skip news πŸ”‡ This change does not require a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants