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

WIP: add please build #1631

Closed
wants to merge 10 commits into from
Closed

Conversation

namtzigla
Copy link

@namtzigla namtzigla commented Feb 8, 2019

This is an attempt to implement please builder system into skaffold.
Because please is very similar with bazel I'm using the the same approach as bazel builder implementation.

This is waiting for couple of PR on pleasings repo(thought-machine/pleasings#21 and thought-machine/pleasings#22) to be merged and it needs some tests added.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

@namtzigla
Copy link
Author

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@googlebot googlebot added cla: yes and removed cla: no labels Feb 8, 2019
@codecov-io
Copy link

codecov-io commented Feb 8, 2019

Codecov Report

Merging #1631 into master will decrease coverage by 0.07%.
The diff coverage is 43.2%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1631      +/-   ##
==========================================
- Coverage    46.9%   46.82%   -0.08%     
==========================================
  Files         119      121       +2     
  Lines        5134     5217      +83     
==========================================
+ Hits         2408     2443      +35     
- Misses       2478     2521      +43     
- Partials      248      253       +5
Impacted Files Coverage Δ
pkg/skaffold/build/local/local.go 48.38% <ø> (-3.34%) ⬇️
cmd/skaffold/app/cmd/diagnose.go 0% <0%> (ø) ⬆️
pkg/skaffold/build/deps.go 0% <0%> (ø) ⬆️
pkg/skaffold/build/local/please.go 0% <0%> (ø)
pkg/skaffold/please/please.go 77.77% <77.77%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5862895...0ac3dc5. Read the comment docs.

@balopat
Copy link
Contributor

balopat commented Feb 9, 2019

Hi @namtzigla, thank you for opening this PR! We are building a plugin system for builders: #1551 - it will be probably ready in a month or so - the please.build builder sounds like a perfect candidate for a community plugin. The code you're writing now is going to be largely reusable - in the end a similar Builder interface needs to be implemented in go. However until then it doesn't make sense for us to take this contribution as is in the skaffold repo. Thank you for your understanding!

@balopat balopat added the !! blocked !! this issue/PR is blocked by another issue label Feb 12, 2019
@namtzigla
Copy link
Author

Hi @balopat,

In regards of closing #1551 and what you described in #1988 I was wondering is there a chance for the please plugin to be integrated into skaffold ?

thanks.

@priyawadhwa
Copy link
Contributor

Hey @namtzigla -- we've decided to go ahead with a more lightweight plan for custom builders, the new plan is described in #2009.

Once this is done, you should be able to plug in a custom build script into the skaffold.yaml & use whatever builder you want! If you have a chance, feel free to take a look at that issue or the WIP PR #1999 and leave any questions you may have.

@namtzigla
Copy link
Author

due to latest developments I'm going to close this

@namtzigla namtzigla closed this Apr 24, 2019
@priyawadhwa
Copy link
Contributor

Hey @namtzigla -- the initial custom builder implementation is out now, if you're still interested in using please.build with skaffold! Here's an example using a custom build script and associated docs. If you end up trying it out, please open an issue if you run into any trouble, and let us know if it works for you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
!! blocked !! this issue/PR is blocked by another issue cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants