The reason I didn't implement this in the first place was because then just anyone could say /lgtm from an alt account and merge their own terrible code. Now that approvers are in place, if I type /lgtm and I'm not an assignee, should k8s-ci-robot assign me and apply lgtm?
I'd be happy to write the code, just want to make this issue for discussion.
@apelisse @kubernetes/sig-contributor-experience-feature-requests
cc @grodrigues3
I still don't think it's a good idea. We want to grow the list of reviewers, we don't want it to be infinite.
If I'm an approver of the code in my PR, I can still get it merged by /lgtm-ing from my alt-account, and completely pass through review.
If I have the permission to assign myself, then assign myself? Is it possible to check that?
@apelisse I'm not sure whether that's actually an issue. If you're listed as an approver for some code, we already assume you're a good actor and won't abuse those privileges.
Agreed, but I still believe we should expect lgtm from someone with experience with the code.
I'm okay with this contingent on:
lgtm their own code/lgtm can only be written by people listed as "reviewers" in an ancestral owners file (i.e. you can only be assigned and lgtm if you are in an owners file as a reviewer)This requires more logic and prow having some understanding of owners files.
Absolutely, the next step forward here would be to have OWNERS logic in prow.
That sounds great to me. After I've finished my current project I'd like to add a git clone to the plugin client that has the PR fetched when they're called.
Yeah, I think we could do tons of sweet things with a copy of the repo.
I commented on this in the PR:
https://github.com/kubernetes/test-infra/pull/2118#issuecomment-285467089
Most helpful comment
@apelisse I'm not sure whether that's actually an issue. If you're listed as an approver for some code, we already assume you're a good actor and won't abuse those privileges.