Skip to content
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

gen-api-reference-docs: fixes for eventing-contrib #1552

Merged
merged 1 commit into from
Jun 28, 2019

Conversation

ahmetb
Copy link
Contributor

@ahmetb ahmetb commented Jun 27, 2019

This updates to a recent version of gen-crd-api-reference-docs and allows
customizing the -api-dir option so that the docs for theeventing-contrib repo
can be generated.

Fixes #1528.

@googlebot googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Jun 27, 2019
@knative-prow-robot knative-prow-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 27, 2019
@samodell samodell removed the request for review from gyliu513 June 28, 2019 00:02
This updates to a recent version of gen-crd-api-reference-docs and allows
customizing the -api-dir option so that the docs for theeventing-contrib repo
can be generated.

Signed-off-by: Ahmet Alp Balkan <[email protected]>
@ahmetb
Copy link
Contributor Author

ahmetb commented Jun 28, 2019

/assign @RichieEscarez

@RichieEscarez
Copy link
Contributor

The build ran perfectly! Im now trying to stage and test the output in my folk.

@ahmetb
Copy link
Contributor Author

ahmetb commented Jun 28, 2019

SGTM. There are some issues like duplicated API Groups (both for eventing and serving) at the beginning of the doc.

It's happening because now there are different Go packages adding types to an apiGroup, so I need to go figure out how to de-dupe them. Should be a non-blocking issue.

@RichieEscarez
Copy link
Contributor

RichieEscarez commented Jun 28, 2019

I found something interesting (not sure if its a Golang thing), but the HTML output is incorrectly picking up a parenthesis from comment:
https://github.com/knative/eventing-contrib/blob/9d20f1c0ceeffd937d70d77abbea8bb1df3c32f3/contrib/gcppubsub/pkg/apis/sources/v1alpha1/gcp_pubsub_types.go#L58

The 404 is caused by a closing parenthesis ):
image

QUESTION: Would whitespace between the URL and closing parenthesis fix this?

@RichieEscarez
Copy link
Contributor

Ill manually remove the duplicate packages and fix the URLs for now. Thanks Ahmet!

image

image

@RichieEscarez
Copy link
Contributor

Re: duplicate Packages in serving: I found that its due to v1alpha1 vs v1beta1 (thus two serving.knative.dev)

@RichieEscarez RichieEscarez mentioned this pull request Jun 28, 2019
@RichieEscarez
Copy link
Contributor

Capturing manual updates:

eventing-contrib-resources.md:

Remove duplicate Packages links and titles + consolidate Resource Types into single list
image
In each section, remove the duplicate title and move the Resource Types into the consolidated list, for example:
image

serving.md:

Qualify v1alpha1 vs v1beta1 (fix Packages list, link IDs, and Section titles)
image
image
image

@ahmetb
Copy link
Contributor Author

ahmetb commented Jun 28, 2019

We might be able to fix the parenthesis issue by tuning the Markdown renderer.

Can you open an issue to the tool?

@RichieEscarez RichieEscarez added approved lgtm Indicates that a PR is ready to be merged. labels Jun 28, 2019
@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: ahmetb

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow-robot knative-prow-robot merged commit eb734da into knative:master Jun 28, 2019
@RichieEscarez
Copy link
Contributor

created #1559 for tracking the current issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot build release 0.7 API docs for knative/eventing-contrib
4 participants