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

Code owner should not can approve it own PRs #17517

Closed
MaxymVlasov opened this issue May 5, 2020 · 4 comments
Closed

Code owner should not can approve it own PRs #17517

MaxymVlasov opened this issue May 5, 2020 · 4 comments
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@MaxymVlasov
Copy link
Member

MaxymVlasov commented May 5, 2020

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:
kubernetes/website#20727

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

@MaxymVlasov MaxymVlasov added the kind/bug Categorizes issue or PR as related to a bug. label May 5, 2020
@alvaroaleman
Copy link
Member

alvaroaleman commented May 5, 2020

This can be disabled via a setting on the approve plugin:

RequireSelfApproval *bool `json:"require_self_approval,omitempty"`

@MaxymVlasov
Copy link
Member Author

@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
Copy link
Member

@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

@MaxymVlasov
Copy link
Member Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

No branches or pull requests

2 participants