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

Refactor workflows to separate different conditions #27

Merged
merged 9 commits into from
Sep 9, 2024

Conversation

suvayu
Copy link
Collaborator

@suvayu suvayu commented Jul 18, 2024

Summary

  • Update actions to latest versions
  • Add flowchart documentation about workflow structure
  • Factor out testing to a reusable workflow. Remove "if skip" checks in between steps, making the logic linear and easier to follow
    • Fix a bug in the windows installation step, earlier it was installing one wheel at a time. This could inadvertently pull in already published version of dependent (spine) packages. Installing with a single pip install *.whl means spine dependencies are satisfied locally, no risk of pulling in unknown versions from PyPI.
  • Factor out build & download to composite local actions, similar to above, this makes the "if skip" condition simpler (in this case reduced to a single if skip: download else: build)

Testing of the action in the toy repo can be found here.

Unfortunately I had to delete the new tags from the example repos (scm*) otherwise it interferes with the unit tests here. I need to come up with a better way to setup the unit tests 🤔.

Deleted tags on scm*:

  • scm: 0.7.3, 0.7.4
  • scm-dep: 0.2.4
  • scm-base: 0.3.3

Related

Supersedes #23

@suvayu suvayu force-pushed the skip-build-step branch from 681d99b to 4c1aed8 Compare July 18, 2024 13:45
@suvayu suvayu mentioned this pull request Aug 31, 2024
The download wheels from PyPI step was inconsistent for 3.8 since
Spine supports Python >=3.8.1.  Remove this problem by switching to
"Build with the newest supported Python" instead of "Build with the
oldest supported Python".  This should be fine, because now the build
process is isolated, and doesn't execute any code from the package.

See:
#31 (comment)
If installed one by one, it pulls in spine dependencies from PyPI
instead of the local wheels.  Depending on the run, this could be
incorrect.
@suvayu suvayu changed the title workflows: factor out testing to be able to use if: Refactor workflows to slearly separate different conditions Sep 7, 2024
@suvayu suvayu changed the title Refactor workflows to slearly separate different conditions Refactor workflows to separate different conditions Sep 7, 2024
@suvayu suvayu requested a review from soininen September 7, 2024 10:26
Copy link
Contributor

@soininen soininen left a comment

Choose a reason for hiding this comment

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

I'm not an expert on GitHub actions, but by a cursory look, the changes seem sound. I guess I'll see how they work in real life next time I need to release new Toolbox :) Nice work!

@suvayu
Copy link
Collaborator Author

suvayu commented Sep 9, 2024

Thanks! I did quite a bit of testing on the toy repo. Actually, that pip install *.whl issue on Windows, I found it since I was using test.pypi.org there! So I'm fairly confident; famous last words 😶‍🌫️ We'll find out

@suvayu suvayu merged commit bc05625 into master Sep 9, 2024
25 checks passed
@suvayu suvayu deleted the skip-build-step branch September 9, 2024 15:46
@suvayu suvayu mentioned this pull request Sep 9, 2024
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