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.

Sign up for GitHub

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

Jump to bottom

📖 Add tutorial for external plugin #3488

Merged
merged 1 commit into from
Aug 3, 2023
Merged

📖 Add tutorial for external plugin #3488

merged 1 commit into from
Aug 3, 2023

Conversation

Eileen-Yu
Copy link
Member

@Eileen-Yu Eileen-Yu commented Jul 6, 2023 •
edited
Loading

Description:

Add docs for the external plugin tutorial.

Motivation:

  1. Guide users to write, install, and use their own external plugins.
  2. Part of the efforts of 🎉 [Google Summer of Code 2023] - Leverage Phase 2 Plugins API to allow creation and usage of external plugins #3309
  3. Record my experiment & tips along with my practice over the external plugin with JavaScript:
    https://github.com/Eileen-Yu/kb-js-plugin

@k8s-ci-robot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 6, 2023
@Eileen-Yu Eileen-Yu force-pushed the docs/external-plugin-tutorial branch from 8983a03 to fcfd794 Compare July 6, 2023 23:24
rashmigottipati
rashmigottipati reviewed Jul 7, 2023
View reviewed changes
docs/book/src/plugins/external-plugins.md Outdated Show resolved Hide resolved
docs/book/src/plugins/external-plugins.md Outdated Show resolved Hide resolved
docs/book/src/plugins/external-plugins.md Outdated Show resolved Hide resolved
rashmigottipati
rashmigottipati reviewed Jul 7, 2023
View reviewed changes
docs/book/src/plugins/external-plugins.md Outdated Show resolved Hide resolved
docs/book/src/plugins/external-plugins.md Outdated Show resolved Hide resolved
docs/book/src/plugins/external-plugins.md Outdated Show resolved Hide resolved
everettraven
everettraven reviewed Jul 10, 2023
View reviewed changes
Copy link
Contributor

@everettraven everettraven left a comment

Choose a reason for hiding this comment

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

Great start @Eileen-Yu ! Here are some comments from my first review pass:

docs/book/src/plugins/external-plugins.md Outdated Show resolved Hide resolved
docs/book/src/plugins/external-plugins.md Outdated Show resolved Hide resolved
docs/book/src/plugins/external-plugins.md Outdated Show resolved Hide resolved
docs/book/src/plugins/external-plugins.md Outdated Show resolved Hide resolved
docs/book/src/plugins/external-plugins.md Outdated Show resolved Hide resolved
docs/book/src/plugins/external-plugins.md Show resolved Hide resolved
@Eileen-Yu Eileen-Yu force-pushed the docs/external-plugin-tutorial branch from 2456af5 to c0962d4 Compare July 14, 2023 23:32
@Eileen-Yu Eileen-Yu marked this pull request as ready for review July 17, 2023 23:22
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 17, 2023
@Eileen-Yu Eileen-Yu force-pushed the docs/external-plugin-tutorial branch from c0962d4 to d438ddc Compare July 17, 2023 23:52
everettraven
everettraven reviewed Jul 24, 2023
View reviewed changes
Copy link
Contributor

@everettraven everettraven left a comment

Choose a reason for hiding this comment

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

This doc is starting to look really good! A few more nits on my second pass:

docs/book/src/plugins/external-plugins.md Outdated Show resolved Hide resolved
docs/book/src/plugins/external-plugins.md Outdated Show resolved Hide resolved
docs/book/src/plugins/external-plugins.md Outdated Show resolved Hide resolved
docs/book/src/plugins/external-plugins.md Outdated Show resolved Hide resolved
docs/book/src/plugins/external-plugins.md Outdated Show resolved Hide resolved
docs/book/src/plugins/external-plugins.md Outdated Show resolved Hide resolved
@Eileen-Yu Eileen-Yu force-pushed the docs/external-plugin-tutorial branch 3 times, most recently from 2191233 to 41c2d1d Compare July 24, 2023 22:37
Kavinjsir
Kavinjsir approved these changes Jul 24, 2023
View reviewed changes
Copy link
Contributor

@Kavinjsir Kavinjsir left a comment

Choose a reason for hiding this comment

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

LGTM!
Left some nits which can or cannot be applied.

docs/book/src/plugins/external-plugins.md Outdated Show resolved Hide resolved
docs/book/src/plugins/external-plugins.md Outdated Show resolved Hide resolved
@Eileen-Yu Eileen-Yu force-pushed the docs/external-plugin-tutorial branch from 4c73f1a to 2333271 Compare July 25, 2023 20:02
rashmigottipati
rashmigottipati approved these changes Jul 25, 2023
View reviewed changes
Copy link
Contributor

@rashmigottipati rashmigottipati left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Nice work @Eileen-Yu, changes look good to me!

LGTM

everettraven
everettraven approved these changes Jul 25, 2023
View reviewed changes
Copy link
Contributor

@everettraven everettraven left a comment

Choose a reason for hiding this comment

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

This looks great @Eileen-Yu ! Thanks for putting this much needed documentation together!

I do have a couple teeny non-blocking nits:

docs/book/src/plugins/external-plugins.md Outdated Show resolved Hide resolved
docs/book/src/plugins/external-plugins.md Outdated Show resolved Hide resolved
@Eileen-Yu @camilamacedo86
@rashmigottipati @everettraven @Kavinjsir
docs: add tutorial for external plugin
d5ccc15 
Co-authored-by: Camila Macedo <[email protected]>
Co-authored-by: Rashmi Gottipati <[email protected]>
Co-authored-by: Bryce Palmer <[email protected]>
Co-authored-by: Tony Jin <[email protected]>
@Eileen-Yu Eileen-Yu force-pushed the docs/external-plugin-tutorial branch from 245643b to d5ccc15 Compare July 25, 2023 22:54
@rashmigottipati
Copy link
Contributor

/test pull-kubebuilder-e2e-k8s-1-27-1

@everettraven
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 1, 2023
@everettraven
Copy link
Contributor

@camilamacedo86 could you give this an approval whenever you have a chance?

camilamacedo86
camilamacedo86 approved these changes Aug 3, 2023
View reviewed changes
Copy link
Member

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86, Eileen-Yu, everettraven, Kavinjsir, rashmigottipati

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 3, 2023
@camilamacedo86
Copy link
Member

/approved

@rashmigottipati
Copy link
Contributor

/retest

@k8s-ci-robot k8s-ci-robot merged commit e94b7f7 into kubernetes-sigs:master Aug 3, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rashmigottipati rashmigottipati rashmigottipati approved these changes

@camilamacedo86 camilamacedo86 camilamacedo86 approved these changes

@Kavinjsir Kavinjsir Kavinjsir approved these changes

@everettraven everettraven everettraven approved these changes

@varshaprasad96 varshaprasad96 Awaiting requested review from varshaprasad96

Assignees

@camilamacedo86 camilamacedo86

@everettraven everettraven

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Eileen-Yu @k8s-ci-robot @rashmigottipati @everettraven @camilamacedo86 @Kavinjsir