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

"Format PR" CI Workflow fails on PRs that come from forks. #5723

Closed
nicktobey opened this issue Apr 10, 2023 · 5 comments
Closed

"Format PR" CI Workflow fails on PRs that come from forks. #5723

nicktobey opened this issue Apr 10, 2023 · 5 comments
Assignees
Labels
automation CI/CD and release automation

Comments

@nicktobey
Copy link
Contributor

(markup for the workflow found here: https://github.com/dolthub/dolt/blob/main/.github/workflows/ci-format-repo.yaml)

The workflow runs actions/checkout@v3 to fetch the ref pointed to by the PR. But if the PR is from a fork, then the ref isn't in the repo and the action fails.

This can be seen in action on this PR: https://github.com/dolthub/dolt/actions/runs/4580694905/workflow?pr=5655

@nicktobey nicktobey added the automation CI/CD and release automation label Apr 10, 2023
@coffeegoddd coffeegoddd self-assigned this Apr 10, 2023
@fulghum
Copy link
Contributor

fulghum commented Apr 17, 2023

This would be a nice one to fix. I missed a CI check failure on a contribution a couple weeks ago because I saw the failure and incorrectly assumed it was this CI step that always fails. My fault for going too fast, but easy to miss failures when you're expecting to see one you have to ignore.

@coffeegoddd
Copy link
Contributor

Not sure how to fix this atm. afaict we're kinda plagued by this issue actions/checkout#455 .

Using pull_request_target event enabling "Allow commits from maintainers" still results in failed commits to the pull request fork branch.

@fulghum
Copy link
Contributor

fulghum commented Apr 20, 2023

Thanks for looking into this one. Maybe the best we can do right now is only run auto-format and verify-formatting on PRs in the same repo, and for forks, only run verify-formatting? If that's doable, that would still be a nice improvement where the CI check wouldn't always fail for all contributions; if verify-formatting does fail, at least contributors could still take care of it manually and get all their CI checks green.

@coffeegoddd
Copy link
Contributor

good idea, ill work on this

@coffeegoddd
Copy link
Contributor

Think this does the right thing #5782, closing this issue for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automation CI/CD and release automation
Projects
None yet
Development

No branches or pull requests

3 participants