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.

Sign up for GitHub

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

Jump to bottom

Unify jibMaven and jibGradle blocks into jib with auto detection #2808

Merged
merged 16 commits into from
Sep 11, 2019
Merged

Unify jibMaven and jibGradle blocks into jib with auto detection #2808

merged 16 commits into from
Sep 11, 2019

Conversation

briandealwis
Copy link
Member

Combines the JibMavenArtifact and JibGradleArtifact (and corresponding jibMaven: and jibGradle: blocks) into a single JibArtifact and jib: block. Jib now guesses the underlying Jib plugin type based on the file contents within the project.

JibArtifact actually has a Type field — a simple int — for explicitly specifying the plugin type, but it is not current exported as it's only been used for tests. We could make this field explicit later, but we'd need to teach the schema generator how to translate enums.

Fixes #1134

@codecov
Copy link

codecov bot commented Sep 4, 2019 •
edited
Loading

Codecov Report

Merging #2808 into master will decrease coverage by 0.02%.
The diff coverage is 71.65%.

Impacted Files Coverage Δ
pkg/skaffold/schema/latest/config.go 100% <ø> (ø) ⬆️
pkg/skaffold/build/local/local.go 33.33% <0%> (-3.59%) ⬇️
pkg/skaffold/build/gcb/types.go 14.7% <0%> (+1.54%) ⬆️
pkg/skaffold/runner/diagnose.go 13.43% <0%> (+0.38%) ⬆️
pkg/skaffold/util/util.go 86.48% <100%> (+0.57%) ⬆️
pkg/skaffold/jib/jib_maven.go 100% <100%> (ø) ⬆️
pkg/skaffold/initializer/init.go 59.14% <100%> (ø) ⬆️
pkg/skaffold/build/gcb/spec.go 100% <100%> (ø) ⬆️
pkg/skaffold/jib/jib_gradle.go 100% <100%> (ø) ⬆️
pkg/skaffold/build/local/jib_maven.go 100% <100%> (ø) ⬆️
... and 5 more

chanseokoh
chanseokoh reviewed Sep 5, 2019
View reviewed changes
Copy link
Member

@chanseokoh chanseokoh left a comment

Choose a reason for hiding this comment

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

LGTM. The changes seem innocuous and relatively straightforward.

docs/content/en/docs/how-tos/builders/_index.md Outdated Show resolved Hide resolved
integration/examples/jib-multimodule/README.adoc Outdated Show resolved Hide resolved
pkg/skaffold/jib/jib.go Show resolved Hide resolved
pkg/skaffold/build/gcb/jib.go Show resolved Hide resolved
loosebazooka
loosebazooka reviewed Sep 5, 2019
View reviewed changes
Copy link
Member

@loosebazooka loosebazooka left a comment

Choose a reason for hiding this comment

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

How does https://skaffold.dev/docs/references/yaml/ get updated?

pkg/skaffold/jib/jib_maven.go Show resolved Hide resolved
pkg/skaffold/schema/v1beta13/upgrade_test.go Show resolved Hide resolved
@briandealwis
Copy link
Member Author

How does https://skaffold.dev/docs/references/yaml/ get updated?

It uses JS to slurp the schema from the site

balopat
balopat reviewed Sep 6, 2019
View reviewed changes
Makefile Outdated Show resolved Hide resolved
balopat
balopat reviewed Sep 6, 2019
View reviewed changes
deprecation-policy.md Outdated Show resolved Hide resolved
@chanseokoh chanseokoh mentioned this pull request Sep 9, 2019
Closed
@balopat balopat added the docs-modifications runs the docs preview service on the given PR label Sep 10, 2019
@container-tools-bot
Copy link

Please visit http://34.94.233.95:1313 to view changes to the docs.

@container-tools-bot container-tools-bot removed the docs-modifications runs the docs preview service on the given PR label Sep 10, 2019
balopat
balopat suggested changes Sep 10, 2019
View reviewed changes
docs/content/en/docs/how-tos/builders/_index.md
@@ -190,13 +186,13 @@ each produce a separate container image.

#### Maven

Copy link
Contributor

@balopat balopat Sep 10, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I would love to see the maven/gradle detection logic documented here for our users.

@balopat balopat added needs-rebase docs-modifications runs the docs preview service on the given PR kokoro:run runs the kokoro jobs on a PR labels Sep 11, 2019
@container-tools-bot
Copy link

Error creating deployment, please see controller logs for details.

@container-tools-bot container-tools-bot removed the docs-modifications runs the docs preview service on the given PR label Sep 11, 2019
@container-tools-bot
Copy link

Please visit http://34.94.71.232:1313 to view changes to the docs.

@container-tools-bot
Copy link

Error creating deployment, please see controller logs for details.

@kokoro-team kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Sep 11, 2019
@balopat balopat added kokoro:run runs the kokoro jobs on a PR and removed needs-rebase labels Sep 11, 2019
balopat
balopat approved these changes Sep 11, 2019
View reviewed changes
Copy link
Contributor

@balopat balopat left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@balopat balopat merged commit 527b897 into GoogleContainerTools:master Sep 11, 2019
This was referenced Oct 1, 2019
Closed
Merged
@briandealwis briandealwis deleted the unified-jib branch November 11, 2019 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chanseokoh chanseokoh chanseokoh left review comments

@loosebazooka loosebazooka loosebazooka left review comments

@balopat balopat balopat approved these changes

@TadCordle TadCordle Awaiting requested review from TadCordle

@dgageot dgageot Awaiting requested review from dgageot

@nkubala nkubala Awaiting requested review from nkubala

@priyawadhwa priyawadhwa Awaiting requested review from priyawadhwa

@tejal29 tejal29 Awaiting requested review from tejal29

Assignees
No one assigned
Labels
cla: yes kokoro:run runs the kokoro jobs on a PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Jib builder should autodetect Maven vs Gradle
7 participants
@briandealwis @container-tools-bot @balopat @loosebazooka @chanseokoh @googlebot @kokoro-team