Skip to content
This repository has been archived by the owner on Sep 24, 2021. It is now read-only.

Fix the name of the image in the github action #79

Merged
merged 1 commit into from
Jul 9, 2019
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/main.workflow
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ action "Docker Registry" {
action "build" {
uses = "actions/docker/cli@master"
needs = ["Docker Registry"]
args = "build -t docker.pkg.github.com/kubernetes-sigs/cluster-api-provider-docker:latest ."
args = "build -t docker.pkg.github.com/kubernetes-sigs/cluster-api-provider-docker/manager:latest ."
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
args = "build -t docker.pkg.github.com/kubernetes-sigs/cluster-api-provider-docker/manager:latest ."
args = "build -t docker.pkg.github.com/kubernetes-sigs/cluster-api-provider-docker/capd-manager:latest ."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's already in the repository name, do you think it's unclear as is?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's clear and would leave the image name as manager

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah i see your note about consistency in slack.

I do not see the consistency between CAPA/CAPI and I think this is a chance to create patterns that are less redundant than what already exist

Copy link
Contributor

Choose a reason for hiding this comment

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

The image is called capd-manager in the publish-manager.sh
Proposing to be consistent w/ the name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it makes sense in publish-manager to keep it as capd-manager because gcr.io repositories are structured as gcr.io/<project-name>/<image-name>:<tag>. Unfortunately the project-name is almost never going to be cluster-api-provider-docker related and thus the prefix capd- makes sense in that context.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am cool w/ that!

}

action "master" {
Expand All @@ -23,5 +23,5 @@ action "master" {
action "push" {
uses = "actions/docker/cli@master"
needs = ["master"]
args = "push docker.pkg.github.com/kubernetes-sigs/cluster-api-provider-docker:latest"
args = "push docker.pkg.github.com/kubernetes-sigs/cluster-api-provider-docker/manager:latest"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
args = "push docker.pkg.github.com/kubernetes-sigs/cluster-api-provider-docker/manager:latest"
args = "push docker.pkg.github.com/kubernetes-sigs/cluster-api-provider-docker/capd-manager:latest"

}