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

Update uploadDockerImage.yml #1034

Merged
merged 1 commit into from
May 2, 2020
Merged

Update uploadDockerImage.yml #1034

merged 1 commit into from
May 2, 2020

Conversation

f-meloni
Copy link
Member

@f-meloni f-meloni commented Apr 22, 2020

I'm assuming DOCKER_GITHUB_PASSWORD is actually a token with write permissions for the Packages of this repo

env:
USERNAME: ${{ secrets.DOCKER_GITHUB_USERNAME }}
- name: Deploy
run: docker push docker.pkg.github.com/danger/danger-js/danger-danger-js:$VERSION
Copy link

@devnoname120 devnoname120 Apr 22, 2020

Choose a reason for hiding this comment

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

Requires auth to download (even public) images. Why not use docker.io/library/ instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I know, I highlighted this in #810 (comment)
But the credentials we have are for GitHub package registry, then I'm fixing the current script, if use or not dockerhub is a separate discussion, and can be done in parallel.

@danger-public
Copy link

Warnings
⚠️

Please add a changelog entry for your changes. You can find it in CHANGELOG.md

Please add your change and name to the master section.

Generated by 🚫 dangerJS against 968d9de

@f-meloni
Copy link
Member Author

f-meloni commented May 2, 2020

@orta are you ok with this? If yes I will update the changelog and merge it

@orta
Copy link
Member

orta commented May 2, 2020

Seems reasonable to me 👍

@orta orta merged commit 6c90cce into master May 2, 2020
@peril-staging
Copy link
Contributor

peril-staging bot commented May 7, 2020

Thanks for the PR @f-meloni.

This PR has been shipped in v10.2.0 - CHANGELOG.

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.

None yet

4 participants