-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Defining a buildConfig #444
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
Conversation
f5d2d3f to
4b5adea
Compare
dev_guide/builds.adoc
Outdated
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.
STI should be first :)
and should be spelled out as Source-To-Image.
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.
also those need to be links or something... "overview: there are 3 types of builds" and then saying nothing about them and moving on to buildconfig isn't very helpful. They are defined over here:
http://docs.openshift.org/latest/architecture/core_objects/builds.html#build-strategies
(the fact that we have two places for build information is a bit of a challenge)
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.
and I just realized we have this thing already....
http://docs.openshift.org/latest/architecture/core_objects/builds.html#buildconfig
4b5adea to
f7ee1b9
Compare
|
@bparees comments addressed |
|
@bparees still I'm not sure if we want to keep http://docs.openshift.org/latest/architecture/core_objects/builds.html#buildconfig |
|
Hmmm, maybe we should move entire contents into the doc you're updating, do the same with routing and leave in Core Objects just the two describing origin and k8s model. |
|
@mfojtik go ahead and get rid of the BuildConfig in the architecture section as part of this PR. You can move the secrets discussion over too. |
|
@bparees yeah, but only after reaching this PR 😉 |
dev_guide/builds.adoc
Outdated
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.
Now in our api it's called just source, I'd rename it to match.
f7ee1b9 to
2f03963
Compare
|
Missing ImageChange trigger here: https://github.com/openshift/openshift-docs/pull/444/files#diff-6226fa3a4f1691fffcba49d8c9e13a0cR38 |
|
good catch @rhcarvalho |
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.
Please update to v1, I'll be taking care of the rest.
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.
v1!
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.
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.
v1beta3! :) (I don't know, probably not since they haven't cut v1 yet)
44b2071 to
37de006
Compare
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.
you're calling it source build here. so the arch doc lost a change or something.
bf48199 to
6b5784e
Compare
|
@soltysh @mfojtik @bparees A lot of these changes are what I was getting to in my PR #443 (moving the example file and the procedures out of here), so I'll wait for this to merge before I go through it any more. One thing: a lot of the cases of STI have been changed to S2I (which I think is infinitely better), but is there a reason not all of them have changed? It'd be great to get this consistent across all of the docs, so making them consistent here is a start. |
|
@bfallonf apologies for any conflicts we're causing you. as for the sti/s2i naming... yeah we're probably getting to the point where we can search and replace, we've been trying to dip our toes into it. I think i'm just not 100% certain STI isn't used legitimately in our api anywhere so i'm wary of screwing up our docs with a global search/replace, but I don't think that is the case. |
|
@bparees I can manage :) I'll wait for this to go through then make my edits to the procedures. As for sti/s2i, who would I ask about that then? Understandable that it might break something. I'm asking for the sake of docs consistency. |
dev_guide/builds.adoc
Outdated
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.
In v1 this should be "GitHub" and below "Generic".
|
This excerpt was not touched by this PR, but since we're improving the docs in general: "A valid |
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.
STI → S2I or Source-to-Image?!
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 prefer Source-To-Image
|
@rhcarvalho I would rather have this merged now and then follow up with @soltysh and you updating the triggers... this PR is holding off another PR's we have here and causing people to do nasty rebases against this. |
6b5784e to
3f98218
Compare
|
I'm ok with merging this as is, since I'm waiting for this PR to merged as well. All the cleaning can be done as a follow-up either by @mfojtik or others interested parties. |
|
Surely. |
Fix missing gemspec
No description provided.