Test-infra: Code owner should not can approve it own PRs

Created on 5 May 2020  路  4Comments  路  Source: kubernetes/test-infra

What happened:
I am owner and I can approve my own PR.

What you expected to happen:
PR creator should not have ability to /approve the PR, regardless is it owner or not.

How to reproduce it (as minimally and precisely as possible):

  1. User is code owner/reviewer in repo.
  2. Create PR.
  3. Send /approve.

Please provide links to example occurrences, if any:
https://github.com/kubernetes/website/pull/20727

Anything else we need to know?:
It can be feature, not bug, but I not think so.

kinbug

Most helpful comment

@alvaroaleman If I understood correctly, RequireSelfApproval setting disable by default, and if be set to true - PR creator must be approve it own PR, otherwise PR not be merged. It more looks like WIP bot / Draft PR Github functionality.

No. By default code owners just autoapprove their own change. The semantic meaning of approve is "I approve of the idea of this change", so if you are a codeowners, you automatically approve of the idea of the change of the code you own.

Folks can fully skip review process on it own PRs if it is owners/reviewers.

No. A /lgtm is still required, you can never lgtm your own change

All 4 comments

@alvaroaleman If I understood correctly, RequireSelfApproval setting disable by default, and if be set to true - PR creator must be approve it own PR, otherwise PR not be merged. It more looks like WIP bot / Draft PR Github functionality.

But I ask about another stuff:
Folks can fully skip review process on it own PRs if it is owners/reviewers.

@alvaroaleman If I understood correctly, RequireSelfApproval setting disable by default, and if be set to true - PR creator must be approve it own PR, otherwise PR not be merged. It more looks like WIP bot / Draft PR Github functionality.

No. By default code owners just autoapprove their own change. The semantic meaning of approve is "I approve of the idea of this change", so if you are a codeowners, you automatically approve of the idea of the change of the code you own.

Folks can fully skip review process on it own PRs if it is owners/reviewers.

No. A /lgtm is still required, you can never lgtm your own change

Yep, I forgot about /lgtm, if I found that I can /lgtm my own PR, I will repopen this Issue.
Thank you!

Was this page helpful?
0 / 5 - 0 ratings