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

Allow to build image with multiple tags #380

Merged
merged 1 commit into from May 19, 2017
Merged

Allow to build image with multiple tags #380

merged 1 commit into from May 19, 2017

Conversation

orzeh
Copy link
Collaborator

@orzeh orzeh commented May 18, 2017

Adds tags property to DockerBuildImage task and deprecates tag property.
To preserve backward compatibility if tag is defined, tags are ignored.
Fix #286

Adds tags property to DockerBuildImage task and deprecates tag.
To preserve backward compatibility if tag is defined, tags are ignored.
Fix #286
@orzeh orzeh requested a review from cdancy May 18, 2017 22:26
@orzeh orzeh added this to the v3.0.8 milestone May 18, 2017
private String imageCreation() {
"""
import com.bmuschko.gradle.docker.tasks.image.Dockerfile
import com.bmuschko.gradle.docker.tasks.image.DockerBuildImage

task dockerfile(type: Dockerfile) {
from 'ubuntu:12.04'
from 'alpine'
Copy link
Collaborator

Choose a reason for hiding this comment

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

+1

} else if (getTags()) {
def tagListString = getTags().collect {"'${it}'"}.join(", ")
logger.quiet "Using tags ${tagListString} for image."
buildImageCmd.withTags(getTags())
Copy link
Collaborator

@cdancy cdancy May 18, 2017

Choose a reason for hiding this comment

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

Can we allow for both? Maybe something like:

def allTags = [] + getTags()
if (getTag()) { allTags << getTag() }
if (allTags) {
    def tagListString = allTags.collect {"'${it}'"}.join(", ")
    logger.quiet "Using tags ${tagListString} for image."
    buildImageCmd.withTags(allTags)
}

May be a better more succinct way to write this but I can def see folks defining a default tag and then possibly through a doFirst block add additional tags at runtime.

Copy link
Collaborator

@cdancy cdancy left a comment

Choose a reason for hiding this comment

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

One minor comment to address about allowing for both. Let me know what you think.

@cdancy cdancy merged commit 9b0dce4 into master May 19, 2017
@cdancy
Copy link
Collaborator

cdancy commented May 19, 2017

I take that back. Doing so is going to promote bad behavior and force us to maintain both ways for longer than is necessary. Merging now.

@cdancy cdancy deleted the MULTIPLE-TAGS branch May 19, 2017 00:38
@orzeh
Copy link
Collaborator Author

orzeh commented May 19, 2017

Doing so is going to promote bad behavior and force us to maintain both ways for longer than is necessary

Exactly!

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.

None yet

2 participants