-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
base: main
Are you sure you want to change the base?
[primer] Create a class for easier comparison of pylint results #6984
Conversation
0915546
to
a54c77e
Compare
Pull Request Test Coverage Report for Build 2528939077
π - Coveralls |
π€ According to the primer, this change has no effect on the checked open source code. π€π This comment was generated for commit a54c77e |
from pylint.testutils._primer.primer_command import Messages, PackageMessages | ||
|
||
|
||
class Comparator: |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
a54c77e
to
c153227
Compare
Type of Changes
Description
Follow up to #6973