-
Notifications
You must be signed in to change notification settings - Fork 657
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
replace dockerhub with github docker registry #4033
Conversation
BTW, I built the image in https://github.com/valhalla/valhalla/actions/runs/4473531846/jobs/7861023336, but I canceled the build or it'd have pushed this branch's image as |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docker command line is already so minimal that it indeed seems like a good idea to forego the "fancy" docker GH action.
Everything else (also) looks reasonable to me.
Jep that worked, 2 brand new images :) https://github.com/valhalla/valhalla/pkgs/container/valhalla Let me revert the test stuff before we merge. |
this is also done @kevinkreiser . The 2 PRs should merge cleanly without conflict BTW, the docker builds take the same time as it took on CircleCI, around 30 mins. |
I’ll also remove the test tag and image |
closes #4029
Let's try with Github Actions, so much easier. On CircleCI it was running with a default machine as well, not the big ones. So probably similar performance. As temp I enabled to build this branch.
Also removes the
run-
prefix as that's not applicable anymore and since people will have to update the URL they're pulling from anyways, we can break this as well IMO.