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: only run linux in PR by default, and run the rest on labels #2184

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ruben-arts
Copy link
Contributor

This PR will do the following:

  • Disable the build and test on all platforms except linux x86_64
  • Allow the developer to enable the other platforms through labels like ci:all ci:win ci:macos.

The fast runners became to expensive to run on every commit and especially windows. Thus this should allow for quick PR's, to save guard ourselfs we leave the auto merge requirements to include all platforms to be tested, otherwise the maintainer can force merge with the risk of breaking main for non linux platforms. So it will then be their problem to fix it.

@ruben-arts ruben-arts added ci:free With this label only the free ci runners are used ci:all Run the tests all platforms labels Oct 2, 2024
@hoxbro
Copy link
Contributor

hoxbro commented Oct 2, 2024

Just a drive-by comment, but for the required jobs which is no longer run by default, you could add a collector at the end and make that the required one

  collector:
    name: Collect Result
    needs: [build, cargo-test]
    if: always()
    runs-on: ubuntu-latest
    steps:
      - name: check for failures
        if: contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled')
        run: echo job failed && exit 1

@ruben-arts
Copy link
Contributor Author

Just a drive-by comment, but for the required jobs which is no longer run by default, you could add a collector at the end and make that the required one

Oh thanks @hoxbro, thats a good idea. The problem we have here is that you can't run based on the "if" through a matrix. I'm planning to rewrite this into different jobs. Or do you have a good idea how we could build this?

@hoxbro
Copy link
Contributor

hoxbro commented Oct 2, 2024

I haven't looked into the details, but you could dynamically create the matrix in another step and unpack it when needed.

I have done something like that here: https://github.com/holoviz/holoviews/blob/519f720ed5e9affbfdb9ac3efed39f7a00e7fef4/.github/workflows/test.yaml#L121

@ruben-arts
Copy link
Contributor Author

@hoxbro The setup job seems like a smart move!

@ruben-arts ruben-arts marked this pull request as draft October 3, 2024 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:all Run the tests all platforms ci:free With this label only the free ci runners are used
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants