Skip to content

Run primer on 3.7 and 3.10#6756

Merged
DanielNoord merged 1 commit into
pylint-dev:mainfrom
DanielNoord:primer-versions
May 31, 2022
Merged

Run primer on 3.7 and 3.10#6756
DanielNoord merged 1 commit into
pylint-dev:mainfrom
DanielNoord:primer-versions

Conversation

@DanielNoord
Copy link
Copy Markdown
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 think it makes most sense to run the primer on the last and least supported versions. Running it on any other doesn't really add much I think.

Besides, running on 3.7 has the additional benefit of testing the old parser (<3.8).

@coveralls
Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 2411214664

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 95.516%

Totals Coverage Status
Change from base Build 2411048352: 0.0%
Covered Lines: 16230
Relevant Lines: 16992

💛 - Coveralls

@jacobtylerwalls
Copy link
Copy Markdown
Member

The comment will just be for 3.10, though?

@DanielNoord
Copy link
Copy Markdown
Collaborator Author

The comment will just be for 3.10, though?

Yeah. I didn't think the rare case where there is a difference between 3.7 and 3.10 in messages emitted would be worth the effort to try and merge output of different versions.
But by running on multiple versions we can (eventually) deprecate/remove the old primer as we now have almost feature-parity.

Copy link
Copy Markdown
Member

@jacobtylerwalls jacobtylerwalls left a comment

Choose a reason for hiding this comment

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

Super. I'd still like to let the current job on main finish first to see if the keras warning goes away. Then we should remove the old primer.

@DanielNoord
Copy link
Copy Markdown
Collaborator Author

Super. I'd still like to let the current job on main finish first to see if the keras warning goes away. Then we should remove the old primer.

Well, the old primer has the benefit of splitting up in different batches. home-assistant has caught multiple fatal errors previously so it's a real shame that we can't add it here.
Unless we find a clean way to have multiple batches in the new primer I think it make sense to leave the batch_one workflow of old primer running.

@jacobtylerwalls
Copy link
Copy Markdown
Member

A --packages argument to the run command?

@DanielNoord
Copy link
Copy Markdown
Collaborator Author

A --packages argument to the run command?

Probably should do --packages-json=? And then make Primer.packages be based on that or something.

Haven't looked at it yet. Might be tricky to download and compare artefacts from different jobs in the same workflow.

@DanielNoord
Copy link
Copy Markdown
Collaborator Author

Note that merging this will likely break all PRs until we get a new successful run as we need a 3.10 output for the commenter to work (and not the 3.8).

@DanielNoord DanielNoord merged commit fbcb2a1 into pylint-dev:main May 31, 2022
@DanielNoord DanielNoord deleted the primer-versions branch May 31, 2022 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants