8000 Fix action by pca006132 · Pull Request #1059 · elalish/manifold · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix action #1059

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

Merged
merged 2 commits into from
Nov 18, 2024
Merged

Fix action #1059

merged 2 commits into from
Nov 18, 2024

Conversation

pca006132
Copy link
Collaborator

Closes #1058.

  1. Simplified python build_wheels.yaml action a bit. The architecture is default to native only, so we don't have to explicitly tell x86_64 to only run x86_64 builds.
  2. Use workflow complete to trigger deployment workflow, so the deployment workflow will wait for the CI workflow to complete. Note that this also means if anything in the CI workflow failed, the deployment workflow will not be triggered. And this is only triggered when the CI workflow is triggered by pushing to the master branch.
  3. Limit artifact search for deployment and publish_npm to artifacts uploaded from actions from the master branch.

Previously, the deployment event runs when we push to master, i.e. when a PR is merged. When there is only one active PR, things are fine because the latest wasm artifact is the expected one. However, when there are multiple events, the CI will search for the last artifact, which may not be the result from the merged PR. We now try to wait until the master branch CI workflow is complete, and limit the artifact search to artifact produced by workflow run from the master branch. This should avoid downloading an incorrect artifact.

For publish_npm, I can't make it wait for the latest master branch without using external actions like https://github.com/marketplace/actions/wait-on-check. And I feel like we probably won't be pushing to master with large changes to wasm binaries and immediately do a publish, so it should be fine...?

@pca006132 pca006132 requested a review from elalish November 18, 2024 16:20
@pca006132
Copy link
Collaborator Author

Btw this is untested, but if it works it should be correct? (assuming the documentation for the download artifact action is correct). Will go to sleep now, hope that everything just work.

Copy link
Owner
@elalish elalish left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

< 8000 details class="details-overlay details-reset position-relative d-inline-block"> Copy link
codecov bot commented Nov 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.69%. Comparing base (d437097) to head (1bb84d5).
Report is 161 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1059      +/-   ##
==========================================
- Coverage   91.84%   84.69%   -7.15%     
==========================================
  Files          37       62      +25     
  Lines        4976     9685    +4709     
  Branches        0     1050    +1050     
==========================================
+ Hits         4570     8203    +3633     
- Misses        406     1482    +1076     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@pca006132
Copy link
Collaborator Author

I just merge it and see how the deployment goes.

@pca006132 pca006132 merged commit a557225 into elalish:master Nov 18, 2024
23 checks passed
@pca006132 pca006132 deleted the fixAction branch November 18, 2024 16:41
@elalish elalish mentioned this pull request Dec 26, 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.

improve wasm deployment
2 participants
0