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

Release with github workflow #170

Closed
wants to merge 2 commits into from
Closed

Conversation

black7375
Copy link
Contributor

image
Issue: #165

I created a workflow that puts Mac into the release.
You don't have to expose the GitHub key.

Here's how to permanently erase files from the Git History:

git filter-branch --tree-filter 'rm -rf ./release-build.sh' HEAD
git filter-branch --prune-empty HEAD
git push origin --force master

@niklasmohrin niklasmohrin requested a review from dbrgn March 27, 2021 14:41
Copy link
Collaborator

@dbrgn dbrgn left a comment

Choose a reason for hiding this comment

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

Thanks @black7375 for this config! Hm, in general this looks solid, and it would be nice if we could build our releases in CI. However, for binary builds that we provide to our users as-is, I'm a bit wary of using third party actions. The thing is that we're a single "docker push" away from our next release being compromised.

@black7375 are you aware of other approaches for uploading releases to GitHub in CI? Maybe there's some kind of bash script that does the same thing, which we could check into this repository after a quick review?

.github/workflows/release.yml Outdated Show resolved Hide resolved
@black7375
Copy link
Contributor Author

I'll check it out in the evening

@black7375
Copy link
Contributor Author

@dbrgn

https://gist.github.com/schell/2fe896953b6728cc3c5d8d5f9f3a17a3
I haven't tested it yet, but it looks good.

@dbrgn
Copy link
Collaborator

dbrgn commented May 10, 2021

That looks pretty nice and maintainable!

@black7375 black7375 force-pushed the master branch 3 times, most recently from 263b51b to 458613e Compare May 11, 2021 02:17
@black7375
Copy link
Contributor Author

black7375 commented May 11, 2021

First of all, upload is successful.

Are there any changes to the Docker configuration?
https://github.com/black7375/tealdeer/runs/2551474861?check_suite_focus=true
Code is the same, but build fails.

@dbrgn
Copy link
Collaborator

dbrgn commented May 11, 2021

Thanks for the update!

I'm not aware of any changes, but this seems to be an OpenSSL linking issue. OpenSSL will get thrown out by #187, so it's best if we wait until that's merged before finishing this.

@dbrgn
Copy link
Collaborator

dbrgn commented May 26, 2021

@black7375 PR #187 is now merged, OpenSSL is no longer a dependency. Could you rebase this PR?

@black7375
Copy link
Contributor Author

Successfully executed fetch.

@dbrgn
Copy link
Collaborator

dbrgn commented May 28, 2021

@black7375 what does that mean? 🙂 you did not push any changes.

@black7375
Copy link
Contributor Author

I ran a fetch updstream on GitHub.
Ah. Did you mean to rebase the commit log?

@dbrgn
Copy link
Collaborator

dbrgn commented May 29, 2021

Yes, it would be great if you could rebase your master branch against the upstream master branch and push the changes (you'll need git push --force). Then, you could push a new tag to your fork, to test whether the builds now succeed. The last few workflows all failed: https://github.com/black7375/tealdeer/actions

If you need help with rebasing, just let me know, happy to help!

@black7375
Copy link
Contributor Author

Yes. I rebase and it all passes.

I don't know why the existing action failed.
I have created a separate release-related action, which shouldn't affect the test.

@dbrgn dbrgn added this to the v1.5.0 milestone Dec 4, 2021
@dbrgn
Copy link
Collaborator

dbrgn commented Dec 31, 2021

Replaced by #240.

@dbrgn dbrgn closed this Dec 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants