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 more CI test platforms #35

Merged
merged 4 commits into from
Aug 10, 2024
Merged

Conversation

nyurik
Copy link
Contributor

@nyurik nyurik commented May 24, 2024

Ensure this action would pass on all available GitHub runners

@ikalnytskyi
Copy link
Owner

Hey @nyurik,

Thank you for the patch and sorry for being unresponsive that long. The patch makes sense to me and I'd be happy to accept it. Let us first figure out how to proceed with broken tests now. See #38 (comment) for details.

@ikalnytskyi
Copy link
Owner

Hey @nyurik,

May I ask you to rebase your change? I believe the broken test is fixed in the master branch.

@@ -18,13 +18,18 @@ jobs:
strategy:
matrix:
os:
- ubuntu-latest
Copy link
Owner

Choose a reason for hiding this comment

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

*-latest runners are tested in the parametrized job. So maybe we can save some CPU cycles by not running them here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thx, done

Ensure this action would pass on all available GitHub runners
- macos-13
- ubuntu-22.04
- ubuntu-24.04
- macos-11
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think macos-11 exist, and this is why the check could not finish. It looks like the only missing platform is ubuntu-24.04, according to the GitHub docs:

https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners/about-github-hosted-runners#standard-github-hosted-runners-for-public-repositories

So I think this patch could be a 1 line change, after all. 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah, they removed 11 i guess. will fix

@ikalnytskyi ikalnytskyi merged commit 4f88640 into ikalnytskyi:master Aug 10, 2024
11 checks passed
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