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

Remove use of github-release #136

Merged
merged 3 commits into from
May 3, 2019

Conversation

simonpasquier
Copy link
Member

@simonpasquier simonpasquier commented Apr 17, 2019

It has been found that the github-release program can leak credentials in the logs when it encounters an error. The upstream project itself isn't maintained actively, the original author recommends one of the forks but 1) it has the same issue and 2) it doesn't offer release binaries.

Looking at we needed from github-release, it isn't that complicated to use directly the GitHub API client instead of relying on an external program.

Closes #4.
Relates to #28 #26

Signed-off-by: Simon Pasquier <spasquie@redhat.com>
Copy link
Contributor

@sdurrheimer sdurrheimer left a comment

Choose a reason for hiding this comment

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

LGTM

Did you manage to test it out ?

Anyway, thanks for working on that. I wanted to do it a long time ago but didn't manage to actually work on it.

@simonpasquier
Copy link
Member Author

I tested with a repository of mine and it worked ok (including overwriting assets). I was wondering whether the --overwrite flag is even needed, maybe it is fine to always overwrite the assets for draft releases.

Anyway, thanks for working on that. I wanted to do it a long time ago but didn't manage to actually work on it.

No problem!

@brian-brazil
Copy link
Contributor

brian-brazil commented Apr 18, 2019 via email

Signed-off-by: Simon Pasquier <spasquie@redhat.com>
@simonpasquier
Copy link
Member Author

I've updated the PR so it will overwrite any asset file if the release is a draft.

Signed-off-by: Simon Pasquier <spasquie@redhat.com>
Copy link
Contributor

@sdurrheimer sdurrheimer left a comment

Choose a reason for hiding this comment

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

👍

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.

Fetch github-release dependency if necessary when using promu release
3 participants