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

Adding support for Cloud Native Buildpacks #3000

Merged
merged 2 commits into from Oct 30, 2019

Conversation

@dgageot
Copy link
Member

dgageot commented Oct 7, 2019

This is an initial implementation that supports Buildpacks as a new artifact type.

@googlebot googlebot added the cla: yes label Oct 7, 2019
@dgageot dgageot force-pushed the dgageot:buildpacks branch 6 times, most recently from cf98e4c to 1da194f Oct 7, 2019
@dgageot dgageot changed the title Adding native support for Cloud Native Buildpacks Adding support for Cloud Native Buildpacks Oct 7, 2019
@codecov

This comment has been minimized.

Copy link

codecov bot commented Oct 7, 2019

Codecov Report

Merging #3000 into master will increase coverage by 0.12%.
The diff coverage is 72.18%.

Impacted Files Coverage Δ
pkg/skaffold/schema/latest/config.go 100% <ø> (ø) ⬆️
pkg/skaffold/schema/v1beta16/upgrade.go 77.77% <ø> (ø) ⬆️
pkg/skaffold/docker/image.go 76.03% <ø> (+0.92%) ⬆️
pkg/skaffold/schema/versions.go 70.83% <ø> (ø) ⬆️
pkg/skaffold/build/local/local.go 31.57% <0%> (-1.76%) ⬇️
pkg/skaffold/build/local/buildpacks.go 0% <0%> (ø)
pkg/skaffold/runner/diagnose.go 12.5% <0%> (-0.36%) ⬇️
pkg/skaffold/schema/v1beta17/config.go 100% <100%> (ø)
pkg/skaffold/schema/defaults/defaults.go 92.35% <100%> (+0.3%) ⬆️
pkg/skaffold/build/buildpacks/dependencies.go 100% <100%> (ø)
... and 16 more
@dgageot dgageot force-pushed the dgageot:buildpacks branch 10 times, most recently from e3eee67 to 23659ff Oct 7, 2019
@dgageot dgageot force-pushed the dgageot:buildpacks branch 10 times, most recently from d807a97 to efde6a5 Oct 16, 2019
@dgageot dgageot force-pushed the dgageot:buildpacks branch 2 times, most recently from bdb8804 to b0b4c3f Oct 17, 2019
@dgageot dgageot marked this pull request as ready for review Oct 17, 2019
Copy link
Member

nkubala left a comment

this is awesome. a few random comments on my first pass, I'll definitely be playing around with this soon.

pkg/skaffold/build/buildpacks/buildpacks.go Outdated Show resolved Hide resolved
pkg/skaffold/build/buildpacks/buildpacks.go Outdated Show resolved Hide resolved
pkg/skaffold/build/buildpacks/lifecycle.go Outdated Show resolved Hide resolved
pkg/skaffold/build/buildpacks/lifecycle.go Outdated Show resolved Hide resolved
pkg/skaffold/docker/containers.go Outdated Show resolved Hide resolved
@dgageot dgageot force-pushed the dgageot:buildpacks branch 3 times, most recently from 23ccc70 to d8fc990 Oct 21, 2019
Copy link
Member

briandealwis left a comment

Some minor comments. Generally LGTM.

}

// Build builds an artifact with Cloud Native Buildpacks:
// https://buildpacks.io/

This comment has been minimized.

Copy link
@briandealwis

briandealwis Oct 22, 2019

Member

Should this also be a package doc comment?

pkg/skaffold/build/buildpacks/lifecycle.go Outdated Show resolved Hide resolved
pkg/skaffold/build/buildpacks/metadata.go Outdated Show resolved Hide resolved
pkg/skaffold/docker/containers.go Outdated Show resolved Hide resolved
pkg/skaffold/schema/latest/config.go Outdated Show resolved Hide resolved
pkg/skaffold/docker/containers.go Outdated Show resolved Hide resolved
@dgageot dgageot force-pushed the dgageot:buildpacks branch from d8fc990 to b14ad95 Oct 23, 2019
Copy link
Member

nkubala left a comment

left a few more comments, I'm feeling pretty good about this in its current state though, I tried it locally and everything works pretty well.

pkg/skaffold/docker/containers.go Show resolved Hide resolved
pkg/skaffold/docker/containers.go Outdated Show resolved Hide resolved
pkg/skaffold/docker/containers.go Show resolved Hide resolved
@dgageot dgageot force-pushed the dgageot:buildpacks branch from f3616c1 to ae429f1 Oct 30, 2019
dgageot added 2 commits Oct 30, 2019
Signed-off-by: David Gageot <david@gageot.net>
Signed-off-by: David Gageot <david@gageot.net>
@dgageot dgageot force-pushed the dgageot:buildpacks branch from ae429f1 to 47e4b76 Oct 30, 2019
@dgageot

This comment has been minimized.

Copy link
Member Author

dgageot commented Oct 30, 2019

@nkubala since your approval, I had to introduce schema v1beta18. I'm going to assume you're ok with that and merge. Thanks everyone for the review.

@dgageot dgageot merged commit b6fa9f2 into GoogleContainerTools:master Oct 30, 2019
3 checks passed
3 checks passed
Travis CI - Pull Request Build Passed
Details
cla/google All necessary CLAs are signed
kokoro CI build successful.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.