Skip to content

Update walkthrough doc to implement guestbook-operator #90

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
Merged
merged 2 commits into from
May 5, 2020
Merged

Update walkthrough doc to implement guestbook-operator #90

merged 2 commits into from
May 5, 2020

Conversation

@atoato88
Copy link
Contributor

@atoato88 atoato88 commented Apr 21, 2020

This PR updates docs/addon/walkthrough/README.md contents to implement guestbook-operator, instead dashboard-operator.

related to: #80

@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. labels Apr 21, 2020
@k8s-ci-robot k8s-ci-robot requested review from droot and justinsb April 21, 2020 07:39
@k8s-ci-robot k8s-ci-robot added sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 21, 2020
@atoato88 atoato88 force-pushed the modify-walkthrough-doc branch 2 times, most recently from 06d85ff to c77a58f Compare April 21, 2020 07:46
atoato88
atoato88 commented Apr 21, 2020
View reviewed changes
Copy link
Contributor Author

@atoato88 atoato88 left a comment

Choose a reason for hiding this comment

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

I added my concern item.

docs/addon/walkthrough/README.md
```

1. Add an instance of the Application CR in your manifest:
2. Add an instance of the Application CR in your manifest:
Copy link
Contributor Author

@atoato88 atoato88 Apr 21, 2020

Choose a reason for hiding this comment

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

I updated this section for guestbook-oprator, but current guestbook-operator doesn't include Aplication CR.
Which is better to implement Application CR on guestbook-operator or omit this section?
Sorry for that I don't know details about Application CR...

Copy link
Contributor

@johnsonj johnsonj May 5, 2020

Choose a reason for hiding this comment

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

This section should walk through configuring the guestbook operator to manage an Application CR. The two reconcile options added (declarative.WithManagedApplication(r.watchLabels), declarative.WithObjectTransform(addon.TransformApplicationFromStatus)) handle this. These updates look good to me.

The Application CR is meant to bubble up high level information/groupings of a set of resources in the cluster. It's managed by sig-apps.

@atoato88 atoato88 force-pushed the modify-walkthrough-doc branch from c77a58f to 254d040 Compare April 21, 2020 23:24
@atoato88
Copy link
Contributor Author

atoato88 commented Apr 27, 2020

/retitle Update walkthrough doc to implement guestbook-operator

@k8s-ci-robot k8s-ci-robot changed the title [WIP] Update walkthrough doc to implement guestbook-operator Update walkthrough doc to implement guestbook-operator Apr 27, 2020
@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 Apr 27, 2020
@johnsonj
Copy link
Contributor

johnsonj commented May 5, 2020

/lgtm
/approve

thank you @atoato88 ! If you have more open questions on the guestbook-operator/application CR please feel free to follow up

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 5, 2020
@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented May 5, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: atoato88, johnsonj

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 May 5, 2020
@k8s-ci-robot k8s-ci-robot merged commit bc9d912 into kubernetes-sigs:master May 5, 2020
@atoato88 atoato88 deleted the modify-walkthrough-doc branch December 12, 2022 00:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@droot droot Awaiting requested review from droot

@justinsb justinsb Awaiting requested review from justinsb

1 more reviewer

@johnsonj johnsonj johnsonj left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@johnsonj johnsonj

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. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. 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.

3 participants

@atoato88 @johnsonj @k8s-ci-robot