-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
📖 Small fixes for getting started overview guide #3726
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: nemethloci The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Welcome @nemethloci! |
Hi @nemethloci. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
These are small documentation fixes for: #3725 |
@@ -208,7 +208,7 @@ return ctrl.Result{RequeueAfter: nextRun.Sub(r.Now())}, nil | |||
|
|||
When a Custom Resource is applied to the cluster, there's a designated controller to manage the Memcached Kind. You can check its reconciliation implemented: | |||
|
|||
From `testdata/project-v4-with-deploy-image/internal/controller/memcached_controller.go`: | |||
From `$GOPATH/memcached-operator/internal/controller/memcached_controller.go`: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have this example in the dir testdata/project-v4-with-deploy-image/internal/controller/memcached_controller.go
we might create a link.
@@ -33,7 +33,7 @@ kubebuilder init --domain=example.com | |||
Next, we'll create a new API responsible for deploying and managing our Memcached solution. In this instance, we will utilize the [Deploy Image Plugin][deploy-image] to get a comprehensive code implementation for our solution. | |||
|
|||
``` | |||
kubebuilder create api --group example.com --version v1alpha1 --kind Memcached --image=memcached:1.4.36-alpine --image-container-command="memcached,-m=64,-o,modern,-v" --image-container-port="11211" --run-as-user="1001" --plugins="deploy-image/v1-alpha" --make=false | |||
kubebuilder create api --group cache --version v1alpha1 --kind Memcached --image=memcached:1.4.36-alpine --image-container-command="memcached,-m=64,-o,modern,-v" --image-container-port="11211" --run-as-user="1001" --plugins="deploy-image/v1-alpha" --make=false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we using cache to generate the example under testdata/project-v4-with-deploy-image/internal/controller/memcached_controller.go
? Could you please check the shell script that we use? If you want to change it please could you either change the testdata shell and re-run make generate to update the results?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution 🥇
For all PRs we need to have a title meaningful and we use emoji to define the type of contribution. I am updating this one and I hope that you do not mind.
I added a couple of nits as well
PR needs rebase. 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. |
Sorry, I made the very poor mistake of not taking a look at existing issues and PRs before submitting #3758. I apologize for duplicating work here. This now needs a rebase, and some of these issues are no longer relevant. If you are able to rebase and find any additional changes, I will be happy to help review and get the PR merged. |
The PR #3758 was merged. However, if you both see that it is still missing points to be addressed, could you please create a new PR to address those or reopen this one if you wish? Therefore, I hope that you do not mind of we close this one. |
Fixes #3725
Small fixes for getting started guide described in the ticket