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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

馃悰 (go/v4): fix readme content and instructions #3628

Merged
merged 1 commit into from Sep 19, 2023
Merged

馃悰 (go/v4): fix readme content and instructions #3628

merged 1 commit into from Sep 19, 2023

Conversation

camilamacedo86
Copy link
Member

Description

  • Fix the readme instructions
  • Ensure that you have a Prerequisites section
  • Remove the KIND info (we should not suggest the k8s vendor)
  • Remove the kubebuilder doc link and commands to update api since it does not seems to make sense in the readme and either if it would be added then, would be required a lot of more details such as how to generate RBAC and etc.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Sep 17, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86

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 approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 17, 2023
@camilamacedo86 camilamacedo86 changed the title 馃悰 fix readme 馃悰 (go/v4): fix readme content and instructions Sep 17, 2023
Sajiyah-Salat
Sajiyah-Salat reviewed Sep 19, 2023
View reviewed changes
Copy link
Contributor

@Sajiyah-Salat Sajiyah-Salat left a comment

Choose a reason for hiding this comment

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

These are simple typo fix issues I have found. Other than that everything looks well
/lgtm

docs/book/src/component-config-tutorial/testdata/project/README.md
1. Install Instances of Custom Resources:
### Prerequisites
- go version v1.20.0+
- docker version 17.03+.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- docker version 17.03+.
- docker version 17.03+

Copy link
Member Author

Choose a reason for hiding this comment

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

I am OK with this one. But you lgtm so, wdyt about doing that in a follow-up?
Could you raise a follow-up with this change?

docs/book/src/component-config-tutorial/testdata/project/README.md
### Prerequisites
- go version v1.20.0+
- docker version 17.03+.
- kubectl version v1.11.3+.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- kubectl version v1.11.3+.
- kubectl version v1.11.3+

Copy link
Member Author

Choose a reason for hiding this comment

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

I am OK with this one. But you lgtm so, wdyt about doing that in a follow-up?
Could you raise a follow-up with this change?

docs/book/src/component-config-tutorial/testdata/project/README.md
- go version v1.20.0+
- docker version 17.03+.
- kubectl version v1.11.3+.
- Access to a Kubernetes v1.11.3+ cluster.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Access to a Kubernetes v1.11.3+ cluster.
- Access to a Kubernetes v1.11.3+ cluster

Copy link
Member Author

Choose a reason for hiding this comment

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

I am OK with this one. But you lgtm so, wdyt about doing that in a follow-up?
Could you raise a follow-up with this change?

docs/book/src/component-config-tutorial/testdata/project/README.md
privileges or be logged in as admin.

**Create instances of your solution**
You can apply the samples (examples) from the config/sample:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
You can apply the samples (examples) from the config/sample:
You can apply the samples (examples) from the config/samples directory:

Copy link
Member Author

Choose a reason for hiding this comment

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

I am OK with this one. But you lgtm so, wdyt about doing that in a follow-up?
Could you raise a follow-up with this change?

docs/book/src/component-config-tutorial/testdata/project/README.md

```sh
make docker-build docker-push IMG=<some-registry>/project:tag
```

3. Deploy the controller to the cluster with the image specified by `IMG`:
**NOTE:** This image ought to be published in the personal registry you specified.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
**NOTE:** This image ought to be published in the personal registry you specified.
**NOTE:** This image should be published in the personal registry you specified

Copy link
Member Author

Choose a reason for hiding this comment

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

Why should be used instead of ought to?

Copy link
Contributor

Choose a reason for hiding this comment

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

Just to keep it more simple.

docs/book/src/component-config-tutorial/testdata/project/README.md

```sh
make docker-build docker-push IMG=<some-registry>/project:tag
```

3. Deploy the controller to the cluster with the image specified by `IMG`:
**NOTE:** This image ought to be published in the personal registry you specified.
And it is required to have access to pull the image from the working environment.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
And it is required to have access to pull the image from the working environment.
and it is required to have access to pull the image from the working environment.

Copy link
Member Author

Choose a reason for hiding this comment

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

I am OK with this one. But you lgtm so, wdyt about doing that in a follow-up?
Could you raise a follow-up with this change?

docs/book/src/component-config-tutorial/testdata/project/README.md

### How it works
This project aims to follow the Kubernetes [Operator pattern](https://kubernetes.io/docs/concepts/extend-kubernetes/operator/).
>**NOTE**: Ensure that the samples has default values to test it out.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
If there is any need to uninstall the CRDs, you can follow this steps:

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this one is not required because yes we have the need to teardown.
that is a common need.

docs/book/src/component-config-tutorial/testdata/project/README.md
```

2. Run your controller (this will run in the foreground, so switch to a new terminal if you want to leave it running):
**Delete the APIs(CRDs) from the cluster:**
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
**Delete the APIs(CRDs) from the cluster:**
**Delete the CRDs from the cluster:**

Copy link
Member Author

Choose a reason for hiding this comment

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

We create apis (the command is create api)
API is a CRD.

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 19, 2023
@k8s-ci-robot k8s-ci-robot merged commit c7c699a into kubernetes-sigs:master Sep 19, 2023
20 checks passed
@camilamacedo86 camilamacedo86 deleted the fix-readme branch September 19, 2023 06:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Sajiyah-Salat Sajiyah-Salat Sajiyah-Salat left review comments

@Kavinjsir Kavinjsir Awaiting requested review from Kavinjsir

@varshaprasad96 varshaprasad96 Awaiting requested review from varshaprasad96

Assignees

@Sajiyah-Salat Sajiyah-Salat

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.

None yet
3 participants
@camilamacedo86 @k8s-ci-robot @Sajiyah-Salat