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

CI: fail with warnings, and several minor tunings to test workflow #325

Merged
merged 1 commit into from
Jul 11, 2024

Conversation

mwtoews
Copy link
Member

@mwtoews mwtoews commented Jul 11, 2024

Notes on the changes:

  • Tests will fail from warnings (-Werror)
  • Use verbose messages with pytest, since this is the primary output to review
  • Change fail-fast: false to run all jobs of a matrix, even if there is one failure
  • Remove the pre-commit workflow job, since pre-commit.ci was recently configured
  • Use conda to install pytest for the conda jobs (not pip)
  • Apt should't install python3-pip, and use apt-get for non-interactive use
  • Remove 'paths' for 'push', so that the tests are re-run after pull-requests are merged to master

Copy link
Contributor

@EwoutH EwoutH left a comment

Choose a reason for hiding this comment

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

Looks good!

@mwtoews mwtoews merged commit fa284a9 into Toblerity:master Jul 11, 2024
20 checks passed
@mwtoews mwtoews deleted the gha-tidy branch July 11, 2024 10:13
@@ -51,7 +51,7 @@ jobs:

- uses: actions/upload-artifact@v4
with:
name: cibw-wheels-${{ matrix.os }}-${{ strategy.job-index }}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand what's happening here fully, I would make sure to test this.

Copy link
Member Author

Choose a reason for hiding this comment

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

The -${{ strategy.job-index }} was an necessary number for the artefact name, e.g. "cibw-wheels-macos-latest-2.zip". So now it becomes just "cibw-wheels-macos-latest.zip". Artefacts can be viewed/downloaded in the GHA Summary, which I sometimes download to inspect or upload to PyPI (hopefully less-so for future releases).

Copy link
Contributor

@EwoutH EwoutH Jul 11, 2024

Choose a reason for hiding this comment

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

Doesn't this give a problem with macOS artifact names? Since cibuildwheel runs both on x86 and Arm runners?

Edit: Nevermind, they already merge them on the cibuildwheel end to a single artifact, so this doesn't change anything.

Copy link
Member Author

Choose a reason for hiding this comment

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

I just checked to see, and the files look good. Each artefact contains several wheel files created from the runner, so there's no conflict.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants