-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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 Create From URL documentation #3353
Conversation
@sspeiche @luciddreamz any thoughts on where in the docs we should put the information for the API for "Run on OpenShift" it doesn't really go in the console customization docs because that is for cluster administrators. |
Put it in the developers section as it's own page. |
@sspeiche, @luciddreamz: what do we want to call this feature? The card describes it as "Run on OpenShift", but I've been referring to it as "Create from URL" as that arguably better describes the functionality. I completely defer to y'all. |
Create from URL sounds like a better description - that or Deploy on OpenShift Button. |
29b2ebe
to
df449b6
Compare
I like what you have, I'd suggest a few updates:
|
good point. i've eliminated "from a preordained namespace" since the next paragraph explains about namespace whitelisting.
i don't think so. @jwforres or @spadgett?
i'm guessing you didn't notice the examples since they weren't in the original screenshot? i've corrected that by adding a second screenshot with the rest of the page.
it might be confusing since the page displays the title and description of the image stream or template being used? |
dev_guide/create_from_url.adoc
Outdated
|Name|Description|Required|Schema|Default | ||
|template|The value of metadata.name as defined in the template to be used.| | ||
true|string| | ||
|templateParamsMap|A JSON parameters map containing the name key value and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this description is really confusing, particularly 'value key value'. Also an example is probably helpful here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nevermind on the example part, i see it below. This description could still be improved though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I struggled with this description and agree it's confusing. Ideas on how to more clearly state?
@rhamilto I would mention that the parameter values need to be correctly encoded. If you're passing things like URLs, it's likely they'll have special characters (like their own query parameters) that need to be URL escaped. |
true|string| | ||
|imageTag|The value spec.tags.name as defined in the image stream to be used.| | ||
true|string| | ||
|namespace|The name of the namespace containing the image stream and image tag |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems redundant to say that it defaults to openshift when you have a whole column dedicated to the Default values
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair point. Will remove the two instances.
Based on my own testing and this comment, I don't think we need to do any encoding given these URLs will be included in HTML. |
@rhamilto I'm not sure I follow what the comment is saying, but for some values, it's not possible for the browser to automatically encode them for you. For instance, a source URL that contains query parameters like http://example.com/code?branch=master&name=myname Using that as the This could a problem with JSON template parameter values that could have characters like |
Another example: Source URLs with a http://example.com/code#master will cause anything from the |
Ah. Thanks for clarifying, @spadgett! I completely overlooked the parenthesis in your initial comment. Apologies. |
528ba3c
to
dcd5424
Compare
@ahardin-rh, please put this on your radar for 3.5. This PR is probably ok as is, but could probably benefit from a writer's touch. |
[rev_history] |
as h3s or h4s when they should have been h2s or h3s).