Skip to content

Fix redundant Jib image flags generated by init#3191

Merged
TadCordle merged 3 commits intoGoogleContainerTools:masterfrom
TadCordle:init-fix
Nov 6, 2019
Merged

Fix redundant Jib image flags generated by init#3191
TadCordle merged 3 commits intoGoogleContainerTools:masterfrom
TadCordle:init-fix

Conversation

@TadCordle
Copy link
Copy Markdown
Contributor

Fixes #3190.

Description

Since the correct -Dimage flag is automatically generated during Jib builds, it's unnecessary (or even error prone) to generate the flags during skaffold init.

Release Notes

Fix unnecessary Jib image flags generated during skaffold init.

},
ImageName: "different-image",
Workspace: filepath.Join("path", "to"),
ArtifactType: latest.ArtifactType{JibArtifact: &latest.JibArtifact{Type: string(JibGradle)}},
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

About #3190 (comment), is it easy to get rid of type: gradle or it is safe to put them?

Copy link
Copy Markdown
Contributor Author

@TadCordle TadCordle Nov 6, 2019

Choose a reason for hiding this comment

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

@briandealwis would know better than I. But I can try some stuff.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It should be safe to remove.

@codecov
Copy link
Copy Markdown

codecov bot commented Nov 6, 2019

Codecov Report

Merging #3191 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted Files Coverage Δ
pkg/skaffold/jib/jib_init.go 88.13% <100%> (-1.42%) ⬇️
pkg/skaffold/util/tar.go 47.87% <0%> (+4.25%) ⬆️

if j.Project != "" {
jibGradle.Project = j.Project
}
jibGradle.Flags = []string{"-Dimage=" + manifestImage}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should have been --image too, I think?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

-Dimage= will work as well, and I know --image doesn't work when put before, say, jibDockerBuild, so I prefer it to --image. It's safer.

@TadCordle TadCordle merged commit 69cd359 into GoogleContainerTools:master Nov 6, 2019
@TadCordle TadCordle deleted the init-fix branch November 6, 2019 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

skaffold.yaml produced by skaffold init --XXenableJibInit could be simpler

5 participants