search

dask / community

For general discussion and community planning. Discussion issues welcome.
20 stars 3 forks source link

Relax GitHub Actions first time contributor approval #191

Closed jacobtomlinson closed 2 years ago

jacobtomlinson commented 2 years ago

A while back GitHub made it so that new contributors cannot trigger GitHub Actions workflows, and a maintainer has to come by and hit "Approve and Run" every time they push a commit to their PR.

Personally I'm finding this very frustrating as it adds friction to reviews.

Today I learned you can make this less strict so it only applied to folks who are brand new to GitHub. I vote we do this. Thoughts?

https://twitter.com/metcalfc/status/1448414192285806592?t=maeChQZTSUh2Ph0YFk-hGA&s=19

GenevieveBuckley commented 2 years ago

I've also found this annoying. I don't have hugely strong opinions either way, I'm happy to let other people decide.

jrbourbeau commented 2 years ago

No strong opinion from me either. I agree having to approve GHA for each commit to a contributors first PR adds some friction. If nobody objections, I'm inclined to give being less strict a try. @jsignell @fjetter thoughts?

jsignell commented 2 years ago

Less strict sounds good to me.

jrbourbeau commented 2 years ago

I just updated this setting for dask/dask and dask/distributed. Hopefully that will help lessen maintainer burden on reviewing PRs for first-time contributors. Other sub-projects should feel free to update as they see fit.

Thanks for bringing this up @jacobtomlinson

GenevieveBuckley commented 2 years ago

I've just changed this for dask-image too (cc @jakirkham for awareness) we'll see how that goes. (I'd be fine with this feature if one approval per pull request was enough, but having to re-approve every individual commit is not great.)

Here's where to find those settings: https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/enabling-features-for-your-repository/managing-github-actions-settings-for-a-repository#configuring-required-approval-for-workflows-from-public-forks

jacobtomlinson commented 2 years ago

I've also done this for dask-kubernetes and dask-cloudprovider.

digger-yu commented 1 year ago

Hello everyone,

It seems that the poll link provided above is no longer accessible. I would like to share some personal thoughts regarding the pre-defined checklist action that requires manual approval from the maintainer for PR to GitHub. While this may seem more secure, it can be inconvenient from the maintainer's perspective and may defeat the purpose of the action.

To address this issue, I suggest that the switch be turned off by default and be manually turned on and approved by the maintainer if needed. This would prevent unnecessary inconvenience for maintainers who may not know how to turn off the switch.

Overall, I believe this approach would be more user-friendly and efficient. Thank you for considering my suggestions.