Skip to content

[buildpacks] Initial support for project.toml #4258

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

Merged
merged 4 commits into from
May 27, 2020

Conversation

dgageot
Copy link
Contributor

@dgageot dgageot commented May 26, 2020

  • Read env vars from project.toml
  • Read buildpacks list from project.toml
  • skaffold init recognizes a project.toml
  • Added a sample

@codecov
Copy link

codecov bot commented May 26, 2020

Codecov Report

Merging #4258 into master will increase coverage by 0.02%.
The diff coverage is 83.78%.

Impacted Files Coverage Δ
pkg/skaffold/build/buildpacks/dependencies.go 100.00% <ø> (ø)
pkg/skaffold/build/buildpacks/project.go 66.66% <66.66%> (ø)
pkg/skaffold/build/buildpacks/init.go 91.30% <100.00%> (-1.01%) ⬇️
pkg/skaffold/build/buildpacks/lifecycle.go 82.66% <100.00%> (+2.66%) ⬆️
...affold/kubernetes/portforward/kubectl_forwarder.go 63.41% <0.00%> (+2.43%) ⬆️

Comment on lines 90 to 92
buildpacks := artifact.Buildpacks
for _, buildpack := range projectDescriptor.Build.Buildpacks {
// TODO(dgageot): Support version and URI.
buildpacks = append(buildpacks, buildpack.ID)
}
Copy link
Member

Choose a reason for hiding this comment

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

So I think the buildpacks specified in the skaffold.yaml should override those in the buildpack.toml. We may have duplicates here too. (And good point about versions too.)

(We should have a test for this resolution process.)

Copy link
Member

@briandealwis briandealwis left a comment

Choose a reason for hiding this comment

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

I think the ReadProjectDescriptor suggestion is simpler, but I leave it to you

dgageot added 4 commits May 27, 2020 12:17
Signed-off-by: David Gageot <[email protected]>
Signed-off-by: David Gageot <[email protected]>
Signed-off-by: David Gageot <[email protected]>
@dgageot dgageot merged commit ccf06fe into GoogleContainerTools:master May 27, 2020
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.

3 participants