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

twisted workflow addition: "I accept the review" #55

Open
glyph opened this issue May 7, 2023 · 4 comments
Open

twisted workflow addition: "I accept the review" #55

glyph opened this issue May 7, 2023 · 4 comments

Comments

@glyph
Copy link

glyph commented May 7, 2023

Before we fully migrated to Github in the Tracocalypse, we had an additional process rule: any contributor with commit access could accept a passing review from someone outside the project, if they deemed that the reviewer in question was operating in good faith and following the process sufficiently that it should count.

The main thing we need members of the project to do is code review, so getting this practice and earning this trust gradually from project members seems like an important step to me and it's weird that Github doesn't have something built-in. Anyway.

Right now, a few admins (mostly just me, probably) still follow this part of the process by using the admin-only red merge button, but it would be nice if the robot could understand the magic phrase "I accept the review" to get an official passing checkmark from the robot iff there is a grey checkmark review (passing from non-member).

@adiroiban
Copy link
Member

So the requirement is something like this.

  • The PR is created by a member of Twisted team
  • The review is done by a non-member of Twisted team.
  • The non-member accepts the PR
  • GitHub will not allow the non-member to do a review
  • chevah-robot will act as a "surrogate" and will perform the "Approve" action using GitHub API (on behalf of the non-member)
  • The review approve comment can contain a text informing that the review was approved in the name of user X

  • If the PR is not created by a member of Twisted team, and a non-member tries to approve it, nothing will happen (an info comment saying that "We don't trust yet your review" can be nice)

@adiroiban
Copy link
Member

Ok. So it looks like non-team member can do a review ... but maybe that review from a non-team member is not accepted by the branch protection... which I hope it's the case

@danuker
Copy link
Contributor

danuker commented May 16, 2024

Branches should be protected - not a lot of people can commit to a repository; unless maybe auto-merge is enabled. chevah-robot will not merge anything by itself.

Whoever has merge access could check, and "I accept the review" could mean the same as the act of merging. Is that adequate?

(an info comment saying that "We don't trust yet your review" can be nice)

If the above solution is good enough, then I think this is what's left of this ticket. It would be nice to get a message that the approval was from someone outside the contributor team.

@glyph
Copy link
Author

glyph commented May 16, 2024

Whoever has merge access could check, and "I accept the review" could mean the same as the act of merging. Is that adequate?

I guess if we move into the "needs-merge" state, reviewers should be checking that queue as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants