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

CONSOLE-3169: Publish console dynamic plugin extension and api docs #11633

Merged
merged 1 commit into from
Jun 20, 2022

Conversation

TheRealJon
Copy link
Member

@TheRealJon TheRealJon commented Jun 2, 2022

Manually publish dynamic plugin SDK API docs and add CI checks to ensure generated files stay up to date.

README.md
Update root README to provide easier navigation to package docs.

frontend/packages/console-dynamic-plugin-sdk/README.md
Update console dynamic plugin SDK README to provide easier navigation to API and Console Extension docs.

frontend/packages/console-dynamic-plugin-sdk/docs
Generate dynamic plugin docs in this dir so that they are not ignored by git and /generated/ is not included in the GitHub Pages URL path.

frontend/packages/console-dynamic-plugin-sdk/scripts/generate-doc.ts
Refactor and simplify GitHub link generation logic

frontend/packages/console-dynamic-plugin-sdk/scripts/package-definitions.ts
Update location of generated docs so that they are copied into dist

test-frontend.sh
Reorder the checks in this file so that the least time-consuming and compute-intensive steps come first, meaning we will fail faster.

https://issues.redhat.com/browse/CONSOLE-3169

@openshift-ci openshift-ci bot requested review from rhamilto and spadgett June 2, 2022 21:12
@openshift-ci openshift-ci bot added component/sdk Related to console-plugin-sdk approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 2, 2022
Copy link
Member

@spadgett spadgett left a comment

Choose a reason for hiding this comment

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

@TheRealJon If we check in the generated doc files, we should add something to CI that makes sure they're updated when things changed (similar to what we do with yarn.lock).

@vojtechszocs @christianvogt @bipuladh FYI

@spadgett
Copy link
Member

spadgett commented Jun 2, 2022

Can we remove the generated folder from gitignore in the SDK package?

@TheRealJon
Copy link
Member Author

@spadgett

@TheRealJon If we check in the generated doc files, we should add something to CI that makes sure they're updated when things changed (similar to what we do with yarn.lock).

Yeah, definitely. I was going to look into this next

Can we remove the generated folder from gitignore in the SDK package?

I opted to move the docs out of the generated folder since those are the only files we need to check in. The other artifacts in the generated folder can still be ignored. I also thought it would be good to keep /generated/ out of the URL path for GitHub Pages.

@TheRealJon TheRealJon changed the title Publish console dynamic plugin extension and api docs CONSOLE-3169: Publish console dynamic plugin extension and api docs Jun 3, 2022
@TheRealJon
Copy link
Member Author

/label tide/merge-method-squash

@openshift-ci openshift-ci bot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Jun 3, 2022
Copy link
Member

@spadgett spadgett left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 3, 2022
@TheRealJon
Copy link
Member Author

/hold

There seems to be an issue with the generation script. It's generating different source code links in CI and local environments.

@openshift-ci openshift-ci bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. and removed lgtm Indicates that a PR is ready to be merged. labels Jun 6, 2022
@TheRealJon
Copy link
Member Author

/hold cancel

I fixed the issue with the link generation logic.

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 9, 2022
Copy link
Member

@spadgett spadgett left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 9, 2022
test-frontend.sh Show resolved Hide resolved
test-frontend.sh Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@andrewballantyne andrewballantyne left a comment

Choose a reason for hiding this comment

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

My original issues were around how we could end up with multiple doc versions. a 4.11 pointing at master would be a bad thing. But it's just the latest docs.

This is a bit of an oddity... GitHub pages is a single version -- the latest. So any link to master, as long as we keep it up to date, will work.

However anyone looking at the READMEs in previous version, they will get an undesired link reference.

I think we should go with this, and start thinking on how we can get the docs to be more future proof.

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jun 14, 2022
@TheRealJon
Copy link
Member Author

QE Approver
/assign @yapei

Docs Approver:
/assign @opayne1

PX Approver:
/assign @RickJWagner

@spadgett @andrewballantyne I've updated all of the github links to the release-4.11 branch. This will need another lgtm.

@RickJWagner
Copy link

/label px-approved

@openshift-ci openshift-ci bot added the px-approved Signifies that Product Support has signed off on this PR label Jun 14, 2022
@RickJWagner RickJWagner removed their assignment Jun 14, 2022
@opayne1
Copy link
Contributor

opayne1 commented Jun 14, 2022

/label docs-approved

@openshift-ci openshift-ci bot added the docs-approved Signifies that Docs has signed off on this PR label Jun 14, 2022
README.md Outdated Show resolved Hide resolved
@TheRealJon TheRealJon force-pushed the CONSOLE-3038 branch 2 times, most recently from da550d7 to a447687 Compare June 16, 2022 18:54
@TheRealJon
Copy link
Member Author

@yapei I've updated per your comment

Copy link
Contributor

@andrewballantyne andrewballantyne left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 16, 2022
@andrewballantyne
Copy link
Contributor

/retest

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jun 17, 2022
@andrewballantyne
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 17, 2022
@andrewballantyne
Copy link
Contributor

Looks like it had an issue with hosting the cluster for e2e -- then crashed frontend with it. Retrying.

/retest

`README.md`
Update root README to provide easier navigation to package docs.

`frontend/packages/console-dynamic-plugin-sdk/README.md`
Update console dynamic plugin SDK README to provide easier navigation to API and Console Extension docs.

`frontend/packages/console-dynamic-plugin-sdk/docs`
Generate dynamic plugin docs in this dir so that they are not ignored by git and /generated/ is not included in the GitHub Pages URL path.

`frontend/packages/console-dynamic-plugin-sdk/scripts/generate-doc.ts`
Refactor and simplify GitHub link generation logic

`frontend/packages/console-dynamic-plugin-sdk/scripts/package-definitions.ts`
Update location of generated docs so that they are copied into dist

`test-frontend.sh`
Reorder the checks in this file so that the least time-consuming and compute-intensive steps come first, meaning we will fail faster.
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jun 17, 2022
@andrewballantyne
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 17, 2022
Copy link
Contributor

@yapei yapei left a comment

Choose a reason for hiding this comment

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

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Jun 20, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 20, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andrewballantyne, spadgett, TheRealJon, yapei, zherman0

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:
  • OWNERS [TheRealJon,spadgett]

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

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 2 against base HEAD 413429e and 8 for PR HEAD 3cdee25 in total

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 20, 2022

@TheRealJon: all tests passed!

Full PR test history. Your PR dashboard.

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.

@openshift-ci openshift-ci bot merged commit 0ce6463 into openshift:master Jun 20, 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. component/sdk Related to console-plugin-sdk docs-approved Signifies that Docs has signed off on this PR lgtm Indicates that a PR is ready to be merged. px-approved Signifies that Product Support has signed off on this PR qe-approved Signifies that QE has signed off on this PR tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants