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

use registry input in image tags #2

Merged
merged 2 commits into from
Oct 17, 2019
Merged

Conversation

squarebracket
Copy link
Contributor

It seems that the registry input wasn't actually being used anywhere, so I added a line to prepend the registry with a / to each inputs.Tags. This way it should build/push for the given registry.

@jerray
Copy link
Owner

jerray commented Oct 1, 2019

You can find its usage in entrypoint.sh. It is only used to login to the docker registry.
I think user should provide full repository name including registry path. Repository name should be same as the result of docker image ls command.

@squarebracket
Copy link
Contributor Author

Hmm. You don't think it's redundant to have the registry both as its own field and in the repository field? Correct me if I'm wrong, but I don't think docker hub allows for several nested levels, so if you prefer to have the registry as part of the repository field (as opposed to completely separate as in this pr), you could simply chop off what's before the first / if there are two or more /s in the repository and use that for the login.

@jerray
Copy link
Owner

jerray commented Oct 10, 2019

I come up with a solution. When registry is provided, check if repository has prefix same as the registry. If not, prepend registry and / to the repository. You should change inputs.Repository other than inputs.Tags.
By the way, we use tab to indent in Golang. You can use gofmt or goimports to format files automatically.

@squarebracket
Copy link
Contributor Author

Changed.

I also added a .editorconfig file so that tabs are used instead of spaces.

@jerray
Copy link
Owner

jerray commented Oct 16, 2019

@squarebracket Thanks. I'll merge this pr later.

@jerray jerray merged commit 5613b40 into jerray:master Oct 17, 2019
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.

2 participants