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.

Sign up for GitHub

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

🧪 Only run "lowest" & "supported" pip in tox for PR/push @ CI #2142

Merged
4 commits merged into from
Dec 16, 2024
Merged

🧪 Only run "lowest" & "supported" pip in tox for PR/push @ CI #2142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
822c3e6
🧪 Stop running pip from `main` on PR/push
webknjaz Dec 4, 2024
fac71fc
🧪🐛 Fix CI reusable workflow conditionals
webknjaz Dec 4, 2024
6e8c21f
🧪 Rename "previous" to "lowest" in tox
webknjaz Dec 4, 2024
a49992b
🧪 Add "supported" pip in tox for PR/push @ CI
webknjaz Dec 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 5 additions & 14 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,24 +61,15 @@ jobs:
pip-version: >-
${{
fromJSON(
github.job_workflow_sha
inputs.cpython-pip-version
&& inputs.cpython-pip-version
|| '["latest", "previous"]'
|| '["supported", "lowest"]'
)
}}
include:
- os: Ubuntu
python-version: >-
${{
github.job_workflow_sha
&& '3.12-dev'
|| '3.8'
}}
pip-version: main
env:
TOXENV: >-
pip${{ matrix.pip-version }}${{
!github.job_workflow_sha
!inputs.cpython-pip-version
&& '-coverage'
|| ''
}}
Expand Down Expand Up @@ -121,7 +112,7 @@ jobs:
run: tox --skip-pkg-install
- name: Upload coverage to Codecov
if: >-
!github.job_workflow_sha
!inputs.cpython-pip-version
uses: codecov/codecov-action@v3
with:
files: ./coverage.xml
Expand Down Expand Up @@ -150,7 +141,7 @@ jobs:
python-version:
- pypy-3.8
pip-version:
- latest
- supported
env:
TOXENV: pip${{ matrix.pip-version }}
steps:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/cron.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,4 @@ jobs:
uses: ./.github/workflows/ci.yml
with:
cpython-pip-version: >-
["main", "latest", "previous"]
["main", "latest", "supported", "lowest"]
9 changes: 5 additions & 4 deletions tox.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
[tox]
envlist =
# NOTE: keep this in sync with the env list in .github/workflows/ci.yml.
py{38,39,310,311,312,py3}-pip{previous,latest,main}-coverage
pip{previous,latest,main}-coverage
pip{previous,latest,main}
py{38,39,310,311,312,py3}-pip{supported,lowest,latest,main}-coverage
pip{supported,lowest,latest,main}-coverage
pip{supported,lowest,latest,main}
Comment on lines +4 to +6
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
py{38,39,310,311,312,py3}-pip{supported,lowest,latest,main}-coverage
pip{supported,lowest,latest,main}-coverage
pip{supported,lowest,latest,main}
py{38,39,310,311,312,py3}-pip-{supported,lowest,latest,main}-coverage
pip-{supported,lowest,latest,main}-coverage
pip-{supported,lowest,latest,main}

Could we add dashes (and of course elsewhere where referenced) to improve readability?

Copy link
Member Author

Choose a reason for hiding this comment

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

Later. See the other comment.

checkqa
readme
skip_missing_interpreters = True
Expand All @@ -14,7 +14,8 @@ extras =
testing
coverage: coverage
deps =
pipprevious: pip==22.2.*
pipsupported: pip==24.2
piplowest: pip==22.2.*
Comment on lines +17 to +18
Copy link
Member

Choose a reason for hiding this comment

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

What does "supported" actually mean here?

pyproject.toml requires pip >= 22.2, so that maps neatly onto piplowest, but the this lowest is also supported.

Is "pipsupported" the highest version we claim to support?

Like, there's a pip 24.3, but we don't claim to support that (yet)?

If so, would piphighest be clearer, and mirror piplowest better?

(and with dashes for readability)

Copy link
Member Author

Choose a reason for hiding this comment

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

What does "supported" actually mean here?

pyproject.toml requires pip >= 22.2, so that maps neatly onto piplowest, but the this lowest is also supported.

Is "pipsupported" the highest version we claim to support?

Like, there's a pip 24.3, but we don't claim to support that (yet)?

Yes, kind of. I just wanted to have something that is pinned in CI. So after this effort, when a PR fails the CI, it's clear that it's not related to something that said PR changed. I don't want to think too much about this. The immediate goal is to unblock development, and it can be refined later.

If so, would piphighest be clearer, and mirror piplowest better?

Maybe, I was considering it. But in order to avoid spending a lot of time overthinking, I've gone with "supported" as "something we run in CI".

(and with dashes for readability)

I was reminded recently that tox makes dash-separated chunks factors, which might not be desired. I thought about it promptly and decided that it's not worthy of being in the scope of this PR and blocking it. But it could be debated separately, once this is complete. I'm currently blocked on getting #2106 to a working state (plus have some work to do in pypi-publish, which is mostly what's distracting me from this PR).

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, sounds good. Let's check this again when the other things are in place 👍

piplatest: pip
pipmain: https://github.com/pypa/pip/archive/main.zip
setenv =
Expand Down
Loading