Skip to content

Conversation

@soltysh
Copy link

@soltysh soltysh commented Jun 15, 2015

@bparees as promised in here I'm updating the description. ptal

@adellape fyi

@bparees
Copy link
Contributor

bparees commented Jun 16, 2015

lgtm for now, but ultimately needs more fleshing out about what is really expected of a custom builder (read the build definition env variable, do whatever you want to do, output artifacts to the build's output location if one is defined, etc)

@soltysh soltysh force-pushed the 492_followup branch 2 times, most recently from 7298c2b to 8966f9b Compare June 16, 2015 09:23
@soltysh
Copy link
Author

soltysh commented Jun 16, 2015

@bparees I've added a few words about how the process should look like, ptal.

@adellape
Copy link
Contributor

Pending ack from @bparees, I'm good with merging this and following up with some minor edits to wording.

Copy link
Contributor

Choose a reason for hiding this comment

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

this really needs to say "push the image to the output location if the build produces an image and if the output location is defined"

our output is not (currently) a generic destination. Whether in the future we want to make it one, or we just say "if you want to output to an ftp server, define an env variable in your buildconfig that the custom builder can look at to know where to upload to" i don't particularly care.

Copy link
Author

Choose a reason for hiding this comment

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

Updated and created issue openshift/origin#3231

Copy link
Contributor

Choose a reason for hiding this comment

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

you're still assuming the custom builder builds an image.

this should be "if the product of the build is a new image, push the image to...."

Copy link
Author

Choose a reason for hiding this comment

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

Is that better now?

@bparees
Copy link
Contributor

bparees commented Jun 16, 2015

lgtm.

@soltysh
Copy link
Author

soltysh commented Jun 16, 2015

@adellape merge?

adellape added a commit that referenced this pull request Jun 16, 2015
Updated custom builder description
@adellape adellape merged commit 0a5a0f4 into openshift:master Jun 16, 2015
@soltysh soltysh deleted the 492_followup branch June 22, 2015 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants