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

Issue 259 - Create GH Actions workflow to replace AppVeyor #303

Merged
merged 18 commits into from
Aug 22, 2023

Conversation

clyde-johnston
Copy link
Contributor

This PR addresses issue #259 to move the AppVeyor workflow to GH Actions. Some differences:

  • GHA workflow includes tests for Python 3.8 and 3.9 on Windows (these can be excluded if required)
  • GHA workflow tests the demo, check_style and docs as graphviz is installed on the Windows runner (this makes Linux and Windows tests the same)

Important: The Windows self-hosted runners differ from the GH Actions runners in that they are not rebuilt after each workflow run. Therefore, there is a chance the Windows runners could become corrupted. A clean installation of the Windows runners can be made from AWS Images if required.

@clyde-johnston clyde-johnston force-pushed the issue-259 branch 6 times, most recently from 935eb98 to 061e38e Compare June 10, 2023 21:27
@clyde-johnston
Copy link
Contributor Author

clyde-johnston commented Jun 10, 2023

I have tested the PYPI_TEST_API_TOKEN_GLOBAL token on the testpypi repository and it works. I have amended the latest commit in this PR to use the PYPI_API_TOKEN_PYCYPHAL token on the pypi repository. This PR is ready to be merged.

Note: The GHA workflow will fail to deploy until the pycyphal version number is incremented.

@clyde-johnston
Copy link
Contributor Author

The pull request event was added to the workflow as requested.

@clyde-johnston
Copy link
Contributor Author

Note: The demo test fails on Ubuntu runners occasionally and randomly with a KeyboardInterrupt error. A rerun of the job usually resolves this.

Copy link
Member

@pavel-kirienko pavel-kirienko left a comment

Choose a reason for hiding this comment

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

Could you please rename the workflow file so that the name indicates that it also runs the test suite aside from publishing the release?

.github/workflows/make-release.yml Outdated Show resolved Hide resolved
.github/workflows/make-release.yml Outdated Show resolved Hide resolved
.github/workflows/make-release.yml Outdated Show resolved Hide resolved
.github/workflows/make-release.yml Outdated Show resolved Hide resolved
@clyde-johnston clyde-johnston force-pushed the issue-259 branch 6 times, most recently from 9e5ef75 to e0de6f3 Compare August 9, 2023 09:53
@clyde-johnston clyde-johnston force-pushed the issue-259 branch 19 times, most recently from a07f49c to 818e030 Compare August 20, 2023 22:17
@coveralls
Copy link

coveralls commented Aug 20, 2023

Coverage Status

coverage: 95.357% (+0.005%) from 95.352% when pulling c56d148 on issue-259 into 01b9a9b on master.

@clyde-johnston
Copy link
Contributor Author

Sonar and Coveralls are now working. This PR is ready for review.

Copy link
Member

@pavel-kirienko pavel-kirienko left a comment

Choose a reason for hiding this comment

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

This looks good but please confirm that the exit status is not ignored on Windows (this issue has been observed in Yakut). Feel free to merge after this is verified/addressed. Thanks!

@pavel-kirienko pavel-kirienko merged commit f83ec3d into master Aug 22, 2023
14 checks passed
@pavel-kirienko pavel-kirienko deleted the issue-259 branch August 22, 2023 08:04
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.

None yet

3 participants