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

Relax PR approval requirement for previous collaborators #33

Open
cpu opened this issue Jan 9, 2024 · 2 comments
Open

Relax PR approval requirement for previous collaborators #33

cpu opened this issue Jan 9, 2024 · 2 comments

Comments

@cpu
Copy link
Member

cpu commented Jan 9, 2024

An idea from djc in discord:

In fact it would be nice if we can relax it even more, like allow the bot to run if we previously merged a PR from the same author

This will probably be a little bit trickier than #32 - I'm not sure if GitHub exposes this state through the API. Some research will be required.

@aochagavia
Copy link
Collaborator

There seems to be an association for this too:

CONTRIBUTOR

Author has previously committed to the repository.

From the GraphQL API docs.

The docs don't explicitly mention whether the "CONTRIBUTOR" association is available to PRs (the docs talk about comments), but I'd expect it to, because it's used by GitHub Actions itself (i.e. the default is to require approval for first-time contributors, but not for someone who successfully contributed in the past).

@cpu
Copy link
Member Author

cpu commented Jan 10, 2024

Djc had this suggestion:

Was thinking you could just run a PR search with is:merged and author facets and check if you get at least one result

Thinking about this more it feels like it could be more trouble than its worth. We don't get that many repeat contributions. When we do, it's not that onerous to either approve the PR or leave the magic comment to provoke the bot.

Implementing this will be a decent chunk of work, and as Ctz pointed out, it's not very hard to get a small PR accepted and then use that increased level of trust to have a malicious branch run on the benchmarking server. Solving that will be a big job and I don't think the level of friction experienced with the current setup motivates doing it.

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

2 participants