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

add note regarding forks #52

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

jdkandersson
Copy link
Collaborator

This PR follows on from this conversation: canonical/operator-workflows#93 (comment)

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.

Copy link
Contributor

@yhaliaw yhaliaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

yanksyoon
yanksyoon previously approved these changes Feb 13, 2023
Copy link
Contributor

@yanksyoon yanksyoon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jdkandersson
Copy link
Collaborator Author

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

Good point, added

Copy link
Contributor

@yhaliaw yhaliaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@weiiwang01
Copy link
Contributor

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
Copy link
Contributor

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.

@arturo-seijas
Copy link
Contributor

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
Copy link
Collaborator Author

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
Copy link
Contributor

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

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

Successfully merging this pull request may close these issues.

9 participants