Last fixes to GitHub actions CI - #1037
Conversation
Signed-off-by: Steffen Vogel <steffen.vogel@opal-rt.com>
Signed-off-by: Steffen Vogel <steffen.vogel@opal-rt.com>
Signed-off-by: Steffen Vogel <steffen.vogel@opal-rt.com>
leonardocarreras
left a comment
leonardocarreras
left a comment
There was a problem hiding this comment.
Hi @stv0g! Thanks, I have some comments, maybe you can have a quick look. Some are matter of preference too.
package-docker.yamlwas not renamed, so that does not run- hardcoding makes more complicated to test from forks, can we use
inputs.container_imagein general or some derivative of it? - there is a pattern
${{ env.DOCKER_IMAGE }}:$vsinputs.container_image, I think it might be nice to make it uniform. I tend now to like moreinputs.container_imageafter reading, but either should work - devcontainer.json might need to change too, or will pull a stale image
node/.devcontainer/devcontainer.json
Line 5 in 88e8a79
- some other smaller things in the individual comments
| @@ -1,12 +1,12 @@ | |||
| # SPDX-FileCopyrightText: 2026 Institute for Automation of Complex Power Systems, RWTH Aachen University | |||
There was a problem hiding this comment.
You may need to rename this file as packaging-container.yaml for it to work, this is why it skipped too many checks
| on: | ||
| workflow_call: | ||
| inputs: | ||
| docker_image: |
There was a problem hiding this comment.
missing container_image
| on: | ||
| workflow_call: | ||
| inputs: | ||
| docker_image: |
There was a problem hiding this comment.
container_image ?
| env: | ||
| REGISTRY: ghcr.io | ||
| DOCKER_IMAGE: ${{ inputs.docker_image }} | ||
| CONTAINER_IMAGE: ghcr.io/villasframework/node |
There was a problem hiding this comment.
CONTAINER_IMAGE: ${{ inputs.container_image }} ? hardcoding breaks the forks
additionally, now there are two patterns, because others drop and use directly inputs.container_image
|
|
||
| env: | ||
| REGISTRY: ghcr.io | ||
| DOCKER_IMAGE: ${{ inputs.docker_image }} |
There was a problem hiding this comment.
CONTAINER_IMAGE: ${{ inputs.container_image }} ?
| runs-on: ubuntu-latest | ||
| container: | ||
| image: ${{ inputs.docker_image }}/dev-fedora:${{ inputs.ref }} | ||
| image: ghcr.io/villasframework/node/dev-fedora:${{ inputs.ref }} |
There was a problem hiding this comment.
hardcoding makes more complicated to test from forks, can we use inputs.container_image?
| runs-on: ubuntu-latest | ||
| container: | ||
| image: ${{ inputs.docker_image }}/dev-fedora:${{ inputs.ref }} | ||
| image: ghcr.io/villasframework/node/dev-fedora:${{ inputs.ref }} |
There was a problem hiding this comment.
hardcoding makes more complicated to test from forks, can we use inputs.container_image?
| token: ${{ github.token }} | ||
| owner: ${{ github.repository_owner }} | ||
| packages: ^villas/node(/.*)?$ | ||
| packages: ^villasframework/node(/.*)?$ |
There was a problem hiding this comment.
without the villasframework/ part, I think
| --insecure-policy \ | ||
| copy "docker-archive:./bundle-docker" \ | ||
| "docker://${{ env.DOCKER_IMAGE }}:${{ steps.ref.outputs.tag }}-nix-${{ matrix.system }}" | ||
| --insecure-policy \ |
There was a problem hiding this comment.
this line got duplicated from 156, likely a typo
| runs-on: ubuntu-latest | ||
| container: | ||
| image: ${{ inputs.docker_image }}/dev-${{ matrix.image_name }}:${{ inputs.ref }} | ||
| image: ${{ inputs.container_image }}-dev-${{ matrix.image_name }}:${{ inputs.ref }} |
There was a problem hiding this comment.
image: ${{ inputs.container_image }}**/**dev-${{ matrix.image_name }}:${{ inputs.ref }} ? prepare creates it with / not - so the image will not be found
3 participants
Changes