search

canonical / is-charms-contributing-guide

The code contributing guide for the IS charms team
Apache License 2.0
2 stars 1 forks source link

add note regarding forks #52

Open jdkandersson opened 1 year ago

jdkandersson commented 1 year ago

This PR follows on from this conversation: https://github.com/canonical/operator-workflows/pull/93#issuecomment-1427182448

It proposes that PRs from forks should be redirected at a separate branch if any of the status checks cannot be run (which means that any repos using operator-workflows need to have this be done). The branch protection can be updated to decline PRs directly from forks using a similar pattern as the required status checks job. PR to be created.

jdkandersson commented 1 year ago

Is there any guidance on naming of these separate branches? It might be good to set a standard naming convention now.

Good point, added

weiiwang01 commented 1 year ago

Actually, I am not a fan of creating a new branch and PR ourself every time an outside contributor wants to contribute. Creating a new branch and new PR that the outside contributor can not edit might take away outside contributor's agency and discourage the outside contributor.

I think another approach for this problem is to use special comments to trigger actions for PR initiated by outside contributors. You can find an example here: https://github.com/orgs/community/discussions/25389

gregory-schiano commented 1 year ago

Actually, I am not a fan of creating a new branch and PR ourself every time an outside contributor wants to contribute. Creating a new branch and new PR that the outside contributor can not edit might take away outside contributor's agency and discourage the outside contributor.

I think another approach for this problem is to use special comments to trigger actions for PR initiated by outside contributors. You can find an example here: https://github.com/orgs/community/discussions/25389

So the process would be:

Few remarks:

Overall I like this approach, and I've seen it multiple times in public repository.

arturo-seijas commented 1 year ago

Actually, I am not a fan of creating a new branch and PR ourself every time an outside contributor wants to contribute. Creating a new branch and new PR that the outside contributor can not edit might take away outside contributor's agency and discourage the outside contributor. I think another approach for this problem is to use special comments to trigger actions for PR initiated by outside contributors. You can find an example here: https://github.com/orgs/community/discussions/25389

So the process would be:

  • A contributor forks the repo
  • Makes a branch and does a change
  • Creates a PR to our main repo
  • This PR will run a limited workflow (one that doesn't requires secrets/permissions)
  • Someone from the team can the workflow steps which couldn't run from the contributor permissions

Few remarks:

  • We'll have to keep the "success checks" enabled, that will lead to workflow concidered as failed when it's triggered by the PR, but will mark the workflow as success when the workflow is triggered by the comment, allowing to merge the PR
  • Whenever a change is push to the branch we'll have to comment to re-run the workflow, couldn't this be done by a bot ?

Overall I like this approach, and I've seen it multiple times in public repository. +1 Sounds like a nice solution.

jdkandersson commented 1 year ago

So the process would be:

  • A contributor forks the repo
  • Makes a branch and does a change
  • Creates a PR to our main repo
  • This PR will run a limited workflow (one that doesn't requires secrets/permissions)
  • Someone from the team can the workflow steps which couldn't run from the contributor permissions

Few remarks:

  • We'll have to keep the "success checks" enabled, that will lead to workflow concidered as failed when it's triggered by the PR, but will mark the workflow as success when the workflow is triggered by the comment, allowing to merge the PR
  • Whenever a change is push to the branch we'll have to comment to re-run the workflow, couldn't this be done by a bot ?

Overall I like this approach, and I've seen it multiple times in public repository.

I agree this is a much better approach. I don't think we can ever have a bot that kicks of the workflow runs automatically in response to community contributions since that gives a path to any developer to execute anything they want to. At that point, we may as well just allow everyone to access the secrets on the repo

gregory-schiano commented 1 year ago

So the process would be:

  • A contributor forks the repo
  • Makes a branch and does a change
  • Creates a PR to our main repo
  • This PR will run a limited workflow (one that doesn't requires secrets/permissions)
  • Someone from the team can the workflow steps which couldn't run from the contributor permissions

Few remarks:

  • We'll have to keep the "success checks" enabled, that will lead to workflow concidered as failed when it's triggered by the PR, but will mark the workflow as success when the workflow is triggered by the comment, allowing to merge the PR
  • Whenever a change is push to the branch we'll have to comment to re-run the workflow, couldn't this be done by a bot ?

Overall I like this approach, and I've seen it multiple times in public repository.

I agree this is a much better approach. I don't think we can ever have a bot that kicks of the workflow runs automatically in response to community contributions since that gives a path to any developer to execute anything they want to. At that point, we may as well just allow everyone to access the secrets on the repo

Yes indeed, and a manual comment is not that bad and forces us to have a look at the PR before launching the full workflow