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

Fix e2e handling for PRs in forks #73

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

Fix e2e handling for PRs in forks #73

wants to merge 2 commits into from

Conversation

jsoref
Copy link
Contributor

@jsoref jsoref commented Jun 19, 2023

What are you trying to accomplish?

Doing work in/from forks should not result in ❌.

What approach did you choose and why?

#24 introduced e2e tests that require all repositories with actions enabled that either update their main branch or receive PRs to their main branch to have a valid and current secrets.REPO_WRITE_TOKEN -- this is frustrating for people who do work in or from forks. PRs from forks do not get access to secrets, and thus they will fail miserably, as in https://github.com/actions/gh-actions-cache/actions/runs/5301791880/jobs/9600582421

I considered adding a fallback, but afaict there's nothing remotely special about what this code is doing, so I merely defined correct permissions which enables this workflow to work out of the box in forks or for PRs to this repo from forks.

I've also made the slack hook conditional on the presence of the slack secret, as I'm not planning to add that secret to my fork(s).

Anything you want to highlight for special attention from reviewers?

The current changes only able PRs to work in forks, they don't allow PRs to work across forks, to do that would require switching from pull_request to pull_request_target which is perhaps scarier.

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
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.

None yet

1 participant