Skip to content

add sample repo annotation to imagestreams#4619

Merged
openshift-bot merged 1 commit intoopenshift:masterfrom
bparees:samples
Sep 11, 2015
Merged

add sample repo annotation to imagestreams#4619
openshift-bot merged 1 commit intoopenshift:masterfrom
bparees:samples

Conversation

@bparees
Copy link
Contributor

@bparees bparees commented Sep 10, 2015

add a sampleRepo annotation to builder imagestreamtags so the UI can offer it as a suggested repo to use when someone is experimenting with the builder image.

@bparees
Copy link
Contributor Author

bparees commented Sep 10, 2015

@jwforres here's your sample repos. note the ruby one doesn't work until @danmcp creates it (I have the content ready).

@smarterclayton "api" review.

@knrc @rcernich you'll want to consider doing something similar for your imagestream definitions (https://github.com/jboss-openshift/application-templates/blob/master/jboss-image-streams.json) now that you'll be allowing them to be used directly as builders. you should also add the builder tag.

@bparees bparees changed the title add sample repo annotation to imagestreams [DO_NOT_MERGE] add sample repo annotation to imagestreams Sep 10, 2015
@jwforres
Copy link
Member

thank you 👍 @spadgett fyi

@rcernich
Copy link

It would be nice if sampleRepo allowed some way to specify context-dir. All of the EAP quickstarts are in a single git repository and not being able to specify the context directory means you have to use new-app --context-dir if you want to instantiate any of those projects. Might it be possible to specify this as a query parameter in the url, e.g.: https://github.com/jboss-developer/jboss-eap-quickstarts#6.4.x?context-dir=kitchensink

@smarterclayton
Copy link
Contributor

"LGTM"

@bparees
Copy link
Contributor Author

bparees commented Sep 11, 2015

@jwforres @smarterclayton how do you feel about @rcernich 's branch+context dir proposal? (I assume the #6.4.x represents a branch, which we'd turn into a git_ref)

@liggitt
Copy link
Contributor

liggitt commented Sep 11, 2015

ah, fragment encoding... that takes me back

@rcernich
Copy link

i think the fragment bit is already coded into the url parsing, but there's no way to specify context directory.

@liggitt
Copy link
Contributor

liggitt commented Sep 11, 2015

right, the entire fragment is treated as the refspec, this would overlay URL semantics on the fragment... does that mean the ?... bits would be query encoded (percent-escaped, etc)?

@rcernich
Copy link

however you want to tackle it. i'm just trying to add a new requirement. ;)

@jwforres
Copy link
Member

We are going to be working on the whole create flow over the next two
sprints. The source url wont be specified on the first page anymore, it
will be on the main form for creating from a builder image (you pick
builder first). So I would much rather go ahead and add optional inputs for
branch/tag and context dir instead of trying to add some magic thing to the
url to parse for the context dir.

Cc @spadgett

however you want to tackle it. i'm just trying to add a new requirement. ;)


Reply to this email directly or view it on GitHub
#4619 (comment).

@bparees
Copy link
Contributor Author

bparees commented Sep 11, 2015

@jwforres are you suggesting we have:
sampleRepo
sampleRef
sampleContextDir

?

@jwforres
Copy link
Member

I'm ok with that, yes its a lot of added annotations, but only when you
actually need them all. They are clearly defined and don't use some made up
URL scheme that we have to explain. I already don't like that we allow for
#ref in the URL which is something we made up, but I'll allow it since we
had it in v2. I dont want to introduce another one.

On Fri, Sep 11, 2015 at 2:45 PM, Ben Parees notifications@github.com
wrote:

@jwforres https://github.com/jwforres are you suggesting we have:
sampleRepo
sampleRef
sampleContextDir

?


Reply to this email directly or view it on GitHub
#4619 (comment).

@bparees
Copy link
Contributor Author

bparees commented Sep 11, 2015

@jwforres ok. i'm fine with that. it means this PR is good as is and @rcernich can use the additional annotations in his templates.

[merge]

@bparees bparees changed the title [DO_NOT_MERGE] add sample repo annotation to imagestreams add sample repo annotation to imagestreams Sep 11, 2015
@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/4956/) (Image: devenv-fedora_2330)

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 25e0131

@openshift-bot
Copy link
Contributor

[Test]ing while waiting on the merge queue

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 25e0131

@rcernich
Copy link

bueno! thanks folks.

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/4956/)

openshift-bot pushed a commit that referenced this pull request Sep 11, 2015
@openshift-bot openshift-bot merged commit 5537893 into openshift:master Sep 11, 2015
@bparees bparees deleted the samples branch September 15, 2015 16:55
@spadgett spadgett mentioned this pull request Sep 15, 2015
20 tasks
@liggitt
Copy link
Contributor

liggitt commented Sep 16, 2015

Is there a reason we're not using https for these?

@smarterclayton
Copy link
Contributor

Should be using https...

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.

6 participants