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

add bors to CI #2710

Closed
davidhewitt opened this issue Oct 28, 2022 · 7 comments · Fixed by #2761
Closed

add bors to CI #2710

davidhewitt opened this issue Oct 28, 2022 · 7 comments · Fixed by #2761

Comments

@davidhewitt
Copy link
Member

Sounds like we are generally in favour of doing this.

@messense - are you planning to do this? Otherwise I might try to have a play next in the next week or so.

@adamreichold
Copy link
Member

adamreichold commented Oct 28, 2022

So which jobs are we running on each PR before running all of them on the main, staging and trying branch.

I'd suggest fmt, clippy, build (stable Rust on all OS, MSRV and nightly on Linux only, latest stable Python only or at most all Python versions on stable Rust and Linux only), and coverage (one OS).

@davidhewitt I just noticed that our 32-bit Windows build still targets CPython 3.10 instead of .11. Is that intentional?

EDIT: Actually, all include: matrix jobs are still on CPython 3.10.

@adamreichold
Copy link
Member

build (stable Rust on all OS, MSRV and nightly on Linux only, latest stable Python only or at most all Python versions on stable Rust and Linux only), and coverage (one OS).

Theoretically going even more minimal, e.g. running the tests for coverage once might also be useful since we will still test the full matrix before merging into main.

@messense
Copy link
Member

are you planning to do this? Otherwise I might try to have a play next in the next week or so.

I'm currently working on PyO3/maturin#1221 and PyO3/maturin#1214, please go ahead if you have time.

@davidhewitt
Copy link
Member Author

Theoretically going even more minimal, e.g. running the tests for coverage once might also be useful since we will still test the full matrix before merging into main.

Yes, I'd be tempted to keep the on-push checks as light as possible, something like run check (not test) for each OS / Python version, and then one run of the test suite on each OS w. latest Python? Should catch most issues and then full regression test run can make sure everything is tip-top shape.

@adamreichold
Copy link
Member

one run of the test suite on each OS w. latest Python?

Sounds reasonable. I would argue to do this with coverage though as this is information we need before approving.

@davidhewitt
Copy link
Member Author

Agreed

@messense
Copy link
Member

messense commented Nov 2, 2022

Build cache will be a problem for pull requests, GitHub Actions cache has restrictions for accessing a cache, PR usually can only access caches from default main branch, but if we don't run workflows on main there will be no cache available.

@bors bors bot closed this as completed in d53baed Nov 22, 2022
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 a pull request may close this issue.

3 participants