Skip to content

Commit 7c1daf3

Browse files
committed
Improve reviewing PRs guide
This PR proposes an update to the PR review guidelines. The proposed change is about the use of the "Request changes" or the "Approve" option when finishing a PR review. Neither of these two options should be encouraged. We may want to encourage reviewers to always use "Comment", because: - "Request changes" status is sticky and unnecessary. Placing a "/hold" should be okay because we do respect opinions from all reviewers/approvers. The "Request changes" status can only be discarded by people with privileges. We see quite a few cases where the reviewer left a "Request changes" mark and forgot to revisit a PR. Such a sticky status is not friendly to contributors or peer reviewers. - The "Approve" option is confusing. It is not considered by the prow as an approval IIUC. The PR has to be approved again even if it has been "approved" this way.
1 parent a9b7331 commit 7c1daf3

File tree

1 file changed

+16
-9
lines changed

1 file changed

+16
-9
lines changed

content/en/docs/contribute/review/reviewing-prs.md

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,8 @@ weight: 10
1010
Anyone can review a documentation pull request. Visit the [pull requests](https://github.com/kubernetes/website/pulls)
1111
section in the Kubernetes website repository to see open pull requests.
1212

13-
Reviewing documentation pull requests is a
14-
great way to introduce yourself to the Kubernetes community.
15-
It helps you learn the code base and build trust with other contributors.
13+
Reviewing documentation pull requests is a great way to introduce yourself to the Kubernetes
14+
community. It helps you learn the code base and build trust with other contributors.
1615

1716
Before reviewing, it's a good idea to:
1817

@@ -28,7 +27,6 @@ Before reviewing, it's a good idea to:
2827

2928
Before you start a review:
3029

31-
3230
- Read the [CNCF Code of Conduct](https://github.com/cncf/foundation/blob/main/code-of-conduct.md)
3331
and ensure that you abide by it at all times.
3432
- Be polite, considerate, and helpful.
@@ -73,6 +71,7 @@ class third,fourth white
7371

7472
Figure 1. Review process steps.
7573

74+
7675
1. Go to [https://github.com/kubernetes/website/pulls](https://github.com/kubernetes/website/pulls).
7776
You see a list of every open pull request against the Kubernetes website and docs.
7877

@@ -103,12 +102,20 @@ Figure 1. Review process steps.
103102
4. Go to the **Files changed** tab to start your review.
104103

105104
1. Click on the `+` symbol beside the line you want to comment on.
106-
1. Fill in any comments you have about the line and click either **Add single comment** (if you
107-
have only one comment to make) or **Start a review** (if you have multiple comments to make).
105+
1. Fill in any comments you have about the line and click either **Add single comment**
106+
(if you have only one comment to make) or **Start a review** (if you have multiple comments to make).
108107
1. When finished, click **Review changes** at the top of the page. Here, you can add
109-
a summary of your review (and leave some positive comments for the contributor!),
110-
approve the PR, comment or request changes as needed. New contributors should always
111-
choose **Comment**.
108+
a summary of your review (and leave some positive comments for the contributor!).
109+
Please always use the "Comment"
110+
111+
- Avoid clicking the "Request changes" button when finishing your review.
112+
If you want to block a PR from being merged before some further changes are made,
113+
you can leave a "/hold" comment.
114+
Mention why you are setting a hold, and optionally specify the conditions under
115+
which the hold can be removed by you or other reviewers.
116+
117+
- Avoid clicking the "Approve" button when finishing your review.
118+
Leaving a "/approve" comment is recommended most of the time.
112119

113120
## Reviewing checklist
114121

0 commit comments

Comments
 (0)