Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the repository’s GitHub Actions workflows to “optimize” CI by upgrading setup-uv usage and attempting to add caching/conditional execution to reduce repeated work.
Changes:
- Introduces a new cache-check job in the Test workflow intended to skip running the test matrix when a cache hit occurs.
- Updates multiple workflows to use
astral-sh/setup-uv@v7withenable-cache: true. - Simplifies the PyPI workflow to build and publish directly with
uv.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| .github/workflows/test.yml | Adds a check-cache job and gates the test job on a cache-hit condition; upgrades setup-uv and adjusts commands. |
| .github/workflows/pypi.yml | Removes separate build/artifact steps; builds and publishes using uv in a single job. |
| .github/workflows/docs.yml | Upgrades setup-uv to v7 and enables uv caching. |
| .github/workflows/coverage.yml | Upgrades setup-uv to v7 and enables uv caching. |
.github/workflows/test.yml
Outdated
| @@ -33,28 +33,34 @@ jobs: | |||
| restore-keys: | | |||
| ${{ runner.os }}-pytest-${{ matrix.python-version }}- | |||
|
|
|||
There was a problem hiding this comment.
The workflow now runs a full matrix check-cache job that only restores .pytest_cache but never runs pytest, so it can't populate/update the cache. Additionally, .pytest_cache isn't a reliable artifact to skip running tests/lint/type-checks in CI. Consider removing the separate cache-check job and instead cache uv deps and pytest cache within the test job to speed up runs while still executing the checks.
| - name: Run type checking | ||
| if: steps.cache-pytest.outputs.cache-hit != 'true' | ||
| run: uvx ty check . --ignore no-matching-overload |
There was a problem hiding this comment.
uv run ty check requires the ty package to be installed in the environment, but uv sync --group dev does not include ty in pyproject.toml dependency groups. This will fail at runtime; either add ty to the dev dependency group or revert to uvx ty ... (or another installation approach) so the command is available.
.github/workflows/test.yml
Outdated
| strategy: | ||
| matrix: | ||
| python-version: ["3.10", "3.11", "3.12", "3.13", "3.14"] | ||
| fail-fast: false |
There was a problem hiding this comment.
The test matrix includes Python 3.10 even though pyproject.toml declares requires-python = ">=3.11", so CI on 3.10 will fail or be misleading. Also consider whether 3.14 is intentionally supported/available on runners; if not, pin the matrix to supported stable versions (3.11+).
| enable-cache: true | ||
|
|
||
| - name: Set up Python | ||
| run: uv python install 3.14 |
There was a problem hiding this comment.
This workflow pins the build environment to Python 3.14 via uv python install 3.14, which may be unavailable/unstable on GitHub runners and isn't required by requires-python (>=3.11). Prefer a stable version you support (e.g., 3.12 or 3.13) for building and publishing.
| run: uv python install 3.14 | |
| run: uv python install 3.12 |
|
|
||
| - name: Publish distribution 📦 to PyPI | ||
| uses: pypa/gh-action-pypi-publish@release/v1 | ||
| run: uv publish |
There was a problem hiding this comment.
The workflow switched from pypa/gh-action-pypi-publish (which handles trusted publishing) to uv publish, but no API token/environment variables are configured. If uv publish isn't configured for OIDC/trusted publishing in this repo, this step will fail to authenticate. Either keep the trusted-publishing action or document/configure the required auth mechanism for uv publish (token env var or OIDC support).
| run: uv publish | |
| uses: pypa/gh-action-pypi-publish@release/v1 |
.github/workflows/test.yml
Outdated
| test: | ||
| runs-on: ubuntu-latest | ||
| needs: check-cache | ||
| if: needs.check-cache.outputs.cache-hit != 'true' | ||
| strategy: |
There was a problem hiding this comment.
test job condition references needs.check-cache.outputs.cache-hit, but check-cache does not define any job outputs. As written, this condition will never reflect the cache action result (likely evaluating to true every time), so the extra check-cache matrix runs for no benefit. Expose cache-hit as a job output (via outputs:) or remove this job and do cache restore/check within test.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
…#48) * Initial plan * Remove check-cache job and consolidate cache into test job Co-authored-by: HEROgold <21345384+HEROgold@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: HEROgold <21345384+HEROgold@users.noreply.github.com>
No description provided.