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

Refactor the primer command to be able to make the comment better later on #6973

Merged
merged 6 commits into from
Jun 20, 2022

Conversation

Pierre-Sassoulas
Copy link
Member

Type of Changes

Type
βœ“ πŸ”¨ Refactoring

Description

Follow-up to #6969

@Pierre-Sassoulas Pierre-Sassoulas added the Maintenance Discussion or action around maintaining pylint or the dev workflow label Jun 18, 2022
@DanielNoord DanielNoord self-requested a review June 18, 2022 07:57
@coveralls
Copy link

coveralls commented Jun 18, 2022

Pull Request Test Coverage Report for Build 2527383993

  • 121 of 176 (68.75%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 95.285%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pylint/testutils/_primer/primer_command.py 14 15 93.33%
pylint/testutils/_primer/primer.py 8 10 80.0%
pylint/testutils/_primer/primer_compare_command.py 79 82 96.34%
pylint/testutils/_primer/primer_prepare_command.py 5 27 18.52%
pylint/testutils/_primer/primer_run_command.py 15 42 35.71%
Totals Coverage Status
Change from base Build 2524474635: 0.02%
Covered Lines: 16592
Relevant Lines: 17413

πŸ’› - Coveralls

@github-actions

This comment has been minimized.

@Pierre-Sassoulas
Copy link
Member Author

I did not test run/prepare because it's very costly (as costly as launching the primer themself). And it's tested elsewhere.

Copy link
Collaborator

@DanielNoord DanielNoord 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 have the time for a full review and this is quite a big change, but I left some early comments.

pylint/testutils/_primer/primer_command.py Outdated Show resolved Hide resolved
pylint/testutils/_primer/primer.py Outdated Show resolved Hide resolved
@Pierre-Sassoulas
Copy link
Member Author

It look large but it's really only a copy paste of the existing code to separate the three commands. I could separate the commits in various PR if that would help.

@DanielNoord
Copy link
Collaborator

It look large but it's really only a copy paste of the existing code to separate the three commands. I could separate the commits in various PR if that would help.

Yeah, it's just that we'll we're taking a look at this I think it makes sense to see if we haven't forgotten anything with the initial implementation. Such as testing the "too-long-comments".

@github-actions

This comment has been minimized.

pylint/testutils/_primer/primer_command.py Outdated Show resolved Hide resolved
pylint/testutils/_primer/primer_compare_command.py Outdated Show resolved Hide resolved
tests/testutils/_primer/test_primer.py Outdated Show resolved Hide resolved
tests/testutils/_primer/test_primer.py Outdated Show resolved Hide resolved
@Pierre-Sassoulas Pierre-Sassoulas force-pushed the refactor-primer branch 2 times, most recently from 1194eec to cf2f1be Compare June 20, 2022 07:13
tests/testutils/_primer/test_primer.py Outdated Show resolved Hide resolved
tests/testutils/_primer/test_primer.py Show resolved Hide resolved
tests/testutils/_primer/test_primer.py Outdated Show resolved Hide resolved
@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 20877ca

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