8000 Create a release file containing submodules by cjmayo · Pull Request #825 · elalish/manifold · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Create a release file containing submodules #825

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 1 commit into from
May 31, 2024

Conversation

cjmayo
Copy link
Contributor
@cjmayo cjmayo commented May 28, 2024

Fixes #783

This does assume the release tag starts with v. If this workflow checked and then exited without creating a file that could be a problem.

Copy link
codecov bot commented May 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.34%. Comparing base (d437097) to head (174be7c).
Report is 35 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #825      +/-   ##
==========================================
- Coverage   91.84%   89.34%   -2.50%     
==========================================
  Files          37       61      +24     
  Lines        4976     8566    +3590     
  Branches        0      939     +939     
==========================================
+ Hits         4570     7653    +3083     
- Misses        406      913     +507     

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

- name: Add file to release
run: |
cd ${{ env.release_name }} || die
gh release upload ${{ github.ref_name }} ../${{ env.release_name }}.tar.gz
Copy link
Owner

Choose a reason for hiding this comment

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

Does this override whatever Github's default release packaging is? Looks great, though I don't have much ability to review or test this. Should we just see what happens on the next release?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It adds an additional file to the list of assets e.g. manifold-2.4.5.tar.gz

Glad you didn't review it - there were a couple of bits that were not so great (updated now), including relating to my initial comment - now it will strip the leading v from the tag only if it exists.
I also added a step to log the sha256sum of the created file.

I have tested it, publishing and creating a new tag in GitHub. So it should be good to go.

Don't know if you have seen but there is some interesting GitHub stuff in development for automatically registering signatures for artifacts without having to manage keys:
https://github.blog/2024-05-02-introducing-artifact-attestations-now-in-public-beta/
But it is beta at the moment.

Copy link
Owner

Choose a reason for hiding this comment

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

Sounds good, I'll trust you. Looks like the only problem is our CI - it started misbehaving yesterday for no apparent reason. I'm hoping M$ fixes it soon, though if you have any ideas what could be randomly canceling our jobs or how to work around it, I'd love to know! I already tried a few things to no avail: #826.

Copy link
Owner

Choose a reason for hiding this comment

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

Looks like the CI started working again - can you prod this PR with a minor commit to trigger the CI? My manual triggering doesn't rerun the randomly cancelled jobs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problem - gave me the opportunity to remove a stray extra space after tar.

I rebased it because the tests have changed. But thinking about it I suppose that wasn't really the point as this PR doesn't change anything. Sorry, at least it is a short one.

Copy link
Owner

Choose a reason for hiding this comment

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

No problem - in general though I prefer no rebasing and no force-pushing. We do squash and merge anyway, so there's no history pollution. Merging makes things much easier to review because it keeps commits separate. Plus it's safer.

@elalish elalish merged commit 5f4059a into elalish:master May 31, 2024
20 checks passed
@cjmayo cjmayo deleted the release_file branch June 2, 2024 18:43
@elalish elalish mentioned this pull request Jun 12, 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.

Manifold 2.4.5 release tar.gz is incomplete
2 participants
0