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

🌱 Fix grammer, spell, and punctuation issues in api description. #318

Conversation

xuezhaojun
Copy link
Member

Summary

The PR aims to enhance the overall quality of the API description section.

@xuezhaojun
Copy link
Member Author

/assign @oafisher

Copy link
Contributor

openshift-ci bot commented Feb 1, 2024

@xuezhaojun: GitHub didn't allow me to assign the following users: oafisher.

Note that only open-cluster-management-io members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign @oafisher

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.

@@ -19,11 +19,11 @@ spec:
schema:
openAPIV3Schema:
description: ManagedClusterSetBinding projects a ManagedClusterSet into a

Choose a reason for hiding this comment

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

Suggestion: ManagedClusterSetBinding projects a ManagedClusterSet into a
certain namespace. You can create a ManagedClusterSetBinding in
a namespace and bind it to a ManagedClusterSet if both have a RBAC rules
to CREATE on the virtual subresource of managedclustersets/bind. Workloads
that you create in the same namespace can only be distributed to ManagedClusters
in ManagedClusterSets that are bound in this namespace by higher-level controllers.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated.

description: "ManagedClusterSet defines a group of ManagedClusters that user's
workload can run on. A workload can be defined to deployed on a ManagedClusterSet,
which mean: 1. The workload can run on any ManagedCluster in the ManagedClusterSet
description: "ManagedClusterSet defines a group of ManagedClusters that users'

Choose a reason for hiding this comment

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

Suggestion: "ManagedClusterSet defines a group of ManagedClusters that you can run
workloads on. You can define a workload to be deployed on a ManagedClusterSet. See the following options for the workload:
- The workload can run on any ManagedCluster in the ManagedClusterSet
- The workload cannot run on any ManagedCluster outside the ManagedClusterSet
- The service exposed by the workload can be shared in any ManagedCluster
in the ManagedClusterSet \n To assign a ManagedCluster to a certain
ManagedClusterSet, add a label with the name cluster.open-cluster-management.io/clusterset
on the ManagedCluster to refer to the ManagedClusterSet. You are not
allowed to add or remove this label on a ManagedCluster unless you have an
RBAC rule to CREATE on a virtual subresource of managedclustersets/join.
To update this label, you must have the permission on both
the old and new ManagedClusterSet."

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated.

@@ -15,14 +15,14 @@ spec:
- name: v1beta1
schema:
openAPIV3Schema:
description: "PlacementDecision indicates a decision from a placement PlacementDecision
should has a label cluster.open-cluster-management.io/placement={placement
description: "PlacementDecision indicates a decision from a placement. PlacementDecision

Choose a reason for hiding this comment

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

Suggestion: "PlacementDecision indicates a decision from a placement. PlacementDecision
must have a cluster.open-cluster-management.io/placement={placement name} label to reference a certain placement. \n If a placement has spec.numberOfClusters
specified, the total number of decisions contained in the status.decisions
of PlacementDecisions must be the same as NumberOfClusters. Otherwise, the
total number of decisions must equal the number of ManagedClusters that
match the placement requirements. \n Some of the decisions might be empty
when there are not enough ManagedClusters to meet the placement requirements."

@@ -35,14 +35,15 @@ spec:
schema:
openAPIV3Schema:
description: "ManagedCluster represents the desired state and current status

Choose a reason for hiding this comment

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

Suggestion: "ManagedCluster represents the desired state and current status
of a managed cluster. ManagedCluster is a cluster-scoped resource. The name
is the cluster UID. \n The cluster join process is a double opt-in
process. See the following join process steps: \n 1. The agent on the managed cluster creates a CSR on the hub
with the cluster UID and agent name. 2. The agent on the managed cluster
creates a ManagedCluster on the hub. 3. The cluster admin on the hub cluster approves
the CSR for the UID and agent name of the ManagedCluster. 4. The cluster
admin sets the spec.acceptClient of the ManagedCluster to true. 5. The cluster
admin on the managed cluster creates a credential of the kubeconfig for
the hub cluster. \n After the hub cluster creates the cluster namespace, the klusterlet agent
on the ManagedCluster pushes the credential to the hub cluster to use against the
kube-apiserver of the ManagedCluster."

@oafischer
Copy link

@xuezhaojun Thanks for making these updates, I've gone through all the descriptions and added my suggestions. I mostly removed passive voice as per IBM style, otherwise they mostly looked good.

Signed-off-by: GitHub <noreply@github.com>
@xuezhaojun
Copy link
Member Author

xuezhaojun commented Feb 8, 2024

@xuezhaojun Thanks for making these updates, I've gone through all the descriptions and added my suggestions. I mostly removed passive voice as per IBM style, otherwise they mostly looked good.

Thanks @oafischer, all suggestions are updated.

@xuezhaojun
Copy link
Member Author

/assign @qiujian16

Improved the descriptions of CRDs.

Copy link
Member

@qiujian16 qiujian16 left a comment

Choose a reason for hiding this comment

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

/approve
/lgtm

@openshift-ci openshift-ci bot added the lgtm label Feb 8, 2024
Copy link
Contributor

openshift-ci bot commented Feb 8, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qiujian16, xuezhaojun

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

@openshift-ci openshift-ci bot added the approved label Feb 8, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit 480f8ab into open-cluster-management-io:main Feb 8, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants