-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add a basic README and stop publishing to Docker Hub #66
base: main
Are you sure you want to change the base?
Conversation
881d3a5
to
7e1e86e
Compare
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.
This does a third change (add a ros-core
variant), but I think it's fine.
7e1e86e
to
e424352
Compare
uses: docker/build-push-action@v5 | ||
with: | ||
context: . | ||
platforms: linux/amd64,linux/arm64 | ||
push: ${{ github.event_name != 'pull_request' }} | ||
build-args: | | ||
BASE_IMAGE_NAME=${{ matrix.base_image_name }} | ||
BASE_IMAGE_TAG=${{ matrix.base_image_tag }} | ||
BASE_IMAGE=${{ matrix.base_image_name }}:${{ matrix.base_image_tag }} |
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.
We have BASE_IMAGE_NAME
and BASE_IMAGE_TAG
instead of BASE_IMAGE
now:
Lines 12 to 18 in 4397556
ARG BASE_IMAGE_NAME | |
# Base Linux distribution version (one of "focal", "jammy", "noble") | |
ARG BASE_IMAGE_TAG | |
FROM "${BASE_IMAGE_NAME}:${BASE_IMAGE_TAG}" |
Never had a readme here, almost nobody even knew this existed or what it's for
Closes #64
Maybe I should separate these changes? But I found that the readme should explain what's actually happening and I didn't want to document something we were to immediately remove.