Generate API documentation using Golang #15788
Conversation
openshift-merge-robot
added
the
size/XXL
|
@jim-minter can you break this PR down into separate commits for the infrastructure changes to how generation is done, and the new generated content? |
|
also @openshift/api-review ptal at these improvements to the api doc generation/output that @jim-minter has made. |
jim-minter
force-pushed
the
rest_api_work
branch
from
2cd2d7b to
f307d53
Compare
|
@bparees done |
jim-minter
force-pushed
the
rest_api_work
branch
from
f307d53 to
3e6d8e1
Compare
|
lgtm but definitely needs sign off from @openshift/api-review imho |
hack/apidocs.go
Outdated
| @@ -0,0 +1,56 @@ | |||
| package main | |||
hack/apidocs.go
Outdated
| return err | ||
| } | ||
|
|
||
| doc, err := loads.JSONSpec("api/swagger-spec/openshift-openapi-spec.json") |
There was a problem hiding this comment.
from a file instead of the server? What does the file combine (for those who don't recall)
There was a problem hiding this comment.
it's a dump of https://localhost:8443/swaggerapi which is already checked into the repo and kept up-to-date via make update/verify.
| l.Lines = buildLines(s, s.Definitions[RefType(&property)], prefix+" ") | ||
| } | ||
| lines = append(lines, l) | ||
| prefix = strings.Replace(prefix, "-", " ", -1) |
There was a problem hiding this comment.
This makes me tingle. Didn't we already use this value? Seems weird to change it once after the first usage
There was a problem hiding this comment.
Yes, it's hacky but I believe it's functionally correct. The intention is to handle the initial hyphens correctly in the pseudo-yaml that is being output here. I didn't come up with a better way.
|
I like the idea. I don't know enough about the history of original to approve it though. |
openshift-merge-robot
added
the
needs-rebase
jim-minter
force-pushed
the
rest_api_work
branch
from
3e6d8e1 to
9f64be9
Compare
jim-minter
removed
the
needs-rebase
|
/lgtm |
openshift-merge-robot
added
the
approved
|
(we're prepared for/anticipating followups to this, but it's a good stake in the ground) |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
jim-minter
force-pushed
the
rest_api_work
branch
from
9f64be9 to
2b91d4d
Compare
openshift-merge-robot
removed
the
lgtm
|
@bparees repushed having just rebased and run hack/update-generated-swagger-spec.sh ; please re-lgtm |
|
/lgtm
…On Wed, Aug 23, 2017 at 10:46 AM, Jim Minter ***@***.***> wrote:
@bparees <https://github.com/bparees> repushed having just rebased and
run hack/update-generated-swagger-spec.sh ; please re-lgtm
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#15788 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEvl3vo3KQKrapMxVNKBodf1bl8NmWW4ks5sbDtRgaJpZM4O4SHz>
.
--
Ben Parees | OpenShift
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bparees, jim-minter The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
/retest |
|
Automatic merge from submit-queue (batch tested with PRs 15870, 15888, 15788, 15907, 15936) |
|
@jim-minter: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
@bparees this is imperfect, but I believe it's a strong enough starting point for further work and a line has to be drawn somewhere. PTAL.