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

Generate API reference docs for Pipeline #4494

Merged
merged 1 commit into from
Aug 1, 2022

Conversation

abayer
Copy link
Contributor

@abayer abayer commented Jan 18, 2022

Changes

This is the first part of addressing #1250 (and tektoncd/website#117).

Using https://github.com/ahmetb/gen-crd-api-reference-docs/, the tool Knative uses for this, plus my PR at ahmetb/gen-crd-api-reference-docs#43 for an issue with pkg/apis/pipeline/pod, this adds generation of docs/pipeline-api.md to hack/update-codegen.sh.

I assume there'll need to be a follow up PR to the website tooling to actually do something with this new file!

/kind documentation

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

  • Docs included if any changes are user facing
  • Tests included if any functionality added or changed
  • Follows the commit message standard
  • Meets the Tekton contributor standards (including
    functionality, content, code)
  • Release notes block below has been filled in or deleted (only if no user facing changes)

Release Notes

NONE

@tekton-robot tekton-robot added release-note-none Denotes a PR that doesnt merit a release note. kind/documentation Categorizes issue or PR as related to documentation. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 18, 2022
@abayer
Copy link
Contributor Author

abayer commented Jan 18, 2022

/assign @skaegi
/assign @vdemeester

@abayer abayer force-pushed the api-reference-docs branch 3 times, most recently from dda844a to 7272f70 Compare January 18, 2022 18:08
@abayer
Copy link
Contributor Author

abayer commented Jan 18, 2022

I've had to hack things around with go.mod - I can't seem to get go mod vendor to work. I keep getting:

github.com/tektoncd/pipeline/cmd/controller imports
	k8s.io/client-go/rest imports
	k8s.io/client-go/plugin/pkg/client/auth/exec imports
	io/fs: malformed module path "io/fs": missing dot in first path element

So I've hardcoded go run -mod=readonly in hack/update-reference-docs.sh. Hopefully that works in CI - it works for me locally, but so did a bunch of other things that failed on CI.

EDIT: Ok, jumped through some hoops (bumping Go to 1.15 mainly) and got it to vendor. Now I need to fix something else.

@abayer abayer force-pushed the api-reference-docs branch 2 times, most recently from 20f994f to c7e43f4 Compare January 18, 2022 18:44
@abayer
Copy link
Contributor Author

abayer commented Jan 18, 2022

/retest

@abayer
Copy link
Contributor Author

abayer commented Jan 18, 2022

/assign @lbernick
/assign @jerop

FYI, this is knocking off another v1 TODO. =)

@tekton-robot
Copy link
Collaborator

@abayer: GitHub didn't allow me to assign the following users: lbernick.

Note that only tektoncd members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign @lbernick
/assign @jerop

FYI, this is knocking off another v1 TODO. =)

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.

@abayer abayer force-pushed the api-reference-docs branch 2 times, most recently from 31b1a7c to 81ed625 Compare January 19, 2022 17:30
@tekton-robot tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 22, 2022
*/

// Package apis contains API Schema definitions for the various API groups
package apis
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need an annotation similar to those in pod/doc.go and run/v1alpha1/doc.go?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't strictly need one, but we could add +k8s:deepcopy-gen=package like in https://github.com/knative/serving/blob/main/pkg/apis/doc.go#L17. I don't think it would actually do anything, though.

@lbernick
Copy link
Member

Awesome, thank you Andrew!

Do you need to also add the annotations to the other CRDs? (e.g. here for Pipeline and similarly for PipelineRun, Task, TaskRun, etc)

@abayer
Copy link
Contributor Author

abayer commented Jan 24, 2022

@lbernick No - pkg/apis/pipeline/pod is a special case, since it's used by pkg/apis/pipeline/v1alpha1 and pkg/apis/pipeline/v1beta1 but is itself unversioned.

@abayer abayer force-pushed the api-reference-docs branch from 81ed625 to a29a3a3 Compare January 24, 2022 15:58
@tekton-robot tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 24, 2022
@lbernick
Copy link
Member

LGTM!

go.mod Outdated
@@ -3,6 +3,7 @@ module github.com/tektoncd/pipeline
go 1.16

require (
github.com/abayer/gen-crd-api-reference-docs v0.999.0
Copy link
Member

@afrittoli afrittoli Feb 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you comment here with a link to your PR https://github.com/ahmetb/gen-crd-api-reference-docs/pull/43/files so we don't forget to switch back once it's merged?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! On it.

@abayer abayer force-pushed the api-reference-docs branch from f079b5c to c9a8b74 Compare March 25, 2022 12:36
@abayer
Copy link
Contributor Author

abayer commented Mar 25, 2022

/retest

@abayer abayer force-pushed the api-reference-docs branch from c9a8b74 to 1161a56 Compare March 29, 2022 16:29
@tekton-robot tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 31, 2022
@abayer abayer force-pushed the api-reference-docs branch from 1161a56 to b820618 Compare March 31, 2022 16:46
@tekton-robot tekton-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 31, 2022
@tekton-robot
Copy link
Collaborator

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale with a justification.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.

@tekton-robot tekton-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 2, 2022
@tekton-robot
Copy link
Collaborator

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten with a justification.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle rotten

Send feedback to tektoncd/plumbing.

@tekton-robot tekton-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Aug 1, 2022
@abayer
Copy link
Contributor Author

abayer commented Aug 1, 2022

/remove-lifecycle rotten

I'm actually working on resurrecting this today. =)

@tekton-robot tekton-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Aug 1, 2022
@abayer abayer force-pushed the api-reference-docs branch from b820618 to 69acedf Compare August 1, 2022 14:38
@tekton-robot tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 1, 2022
@tekton-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vdemeester

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

@abayer
Copy link
Contributor Author

abayer commented Aug 1, 2022

/cc @afrittoli @lbernick - I think this should work/be ready now, using https://github.com/tektoncd/ahmetb-gen-crd-api-reference-docs.

@abayer
Copy link
Contributor Author

abayer commented Aug 1, 2022

/retest

@@ -149,3 +151,5 @@ require (
sigs.k8s.io/json v0.0.0-20211208200746-9f7c6b3444d2 // indirect
sigs.k8s.io/structured-merge-diff/v4 v4.2.1 // indirect
)

replace github.com/ahmetb/gen-crd-api-reference-docs => github.com/tektoncd/ahmetb-gen-crd-api-reference-docs v0.3.1-0.20220729140133-6ce2d5aafcb4 // Waiting for https://github.com/ahmetb/gen-crd-api-reference-docs/pull/43/files to merge
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the comment here still relevant? do you need the replace?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, because the package in the forked repo is still github.com/ahmetb/gen-crd-api-reference-docs.

set -o nounset

echo "Generating API reference docs ..."
# TODO(abayer): Switch to github.com/ahmetb/gen-crd-api-reference-docs when https://github.com/ahmetb/gen-crd-api-reference-docs/pull/43 is merged/vendor is updated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this comment can be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, I forgot that.

@@ -0,0 +1,42 @@
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would you mind adding a readme in /hack that explains a bit what these files you're adding are for, and link to any reference docs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lemme see what I can remember. =)

This is the first part of addressing tektoncd#1250 (and tektoncd/website#117).

Using https://github.com/ahmetb/gen-crd-api-reference-docs/, the tool Knative uses for this, plus my PR at ahmetb/gen-crd-api-reference-docs#43, forked to https://github.com/tektoncd/ahmetb-gen-crd-api-reference-docs, for an issue with `pkg/apis/pipeline/pod`, this adds generation of `docs/pipeline-api.md` to `hack/update-codegen.sh`.

Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>
@abayer abayer force-pushed the api-reference-docs branch from 69acedf to 6d31e0f Compare August 1, 2022 14:52
@@ -9,4 +9,7 @@ Tekton Pipelines.
- [`update-openapigen.sh`](./update-openapigen.sh): Updates OpenAPI specification and Swagger file.
- [`verify-codegen.sh`](./verify-codegen.sh): Verifies that auto-generated
client libraries are up-to-date.
- [`update-reference-docs.sh`](./update-reference-docs.sh) and related files: Generates [`docs/pipeline-api.md`](../docs/pipeline-api.md).
- [`reference-docs-gen-config.json`](./reference-docs-gen-config.json) is the configuration file for the [`gen-api-reference-docs`](https://github.com/tektoncd/ahmetb-gen-crd-api-reference-docs) binary.
- [`reference-docs-template`](./reference-docs-template) contains Go templates for the generated Markdown.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lbernick Is this (and above) what you were looking for?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure yeah this works!

Copy link
Member

@lbernick lbernick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@@ -9,4 +9,7 @@ Tekton Pipelines.
- [`update-openapigen.sh`](./update-openapigen.sh): Updates OpenAPI specification and Swagger file.
- [`verify-codegen.sh`](./verify-codegen.sh): Verifies that auto-generated
client libraries are up-to-date.
- [`update-reference-docs.sh`](./update-reference-docs.sh) and related files: Generates [`docs/pipeline-api.md`](../docs/pipeline-api.md).
- [`reference-docs-gen-config.json`](./reference-docs-gen-config.json) is the configuration file for the [`gen-api-reference-docs`](https://github.com/tektoncd/ahmetb-gen-crd-api-reference-docs) binary.
- [`reference-docs-template`](./reference-docs-template) contains Go templates for the generated Markdown.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure yeah this works!

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 1, 2022
@tekton-robot tekton-robot merged commit 88fbc86 into tektoncd:main Aug 1, 2022
@abayer abayer mentioned this pull request Aug 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/documentation Categorizes issue or PR as related to documentation. lgtm Indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesnt merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants