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

Handle check_suite/check_run from forks #105

Closed
wants to merge 2 commits into from

Conversation

michaelbeaumont
Copy link
Contributor

@michaelbeaumont michaelbeaumont commented Aug 25, 2020

Right now, PRs from forks can't trigger the action because head_branch is null for forks.
This PR works around that by searching open PRs for the same head_sha.
See #46.

lib/api.js Outdated
Comment on lines 122 to 132
logger.info("Could not find branch name in this status check result");
const foundPR = await checkPullRequestsForHeadSha(
context,
event.repository,
payload.head_sha
);
if (!foundPR) {
logger.info(
"Could not find branch name in this status check result" +
" or corresponding PR from a forked repository"
);
}
Copy link
Owner

Choose a reason for hiding this comment

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

I think this logic should all be handled in checkPullRequestsForHeadSha() directly so that the handling is symmetric to the if case (line 120)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@pascalgn
Copy link
Owner

@michaelbeaumont I am closing this now, but only to have a better overview over active PRs! If you're still working on it, feel very free to reopen this PR! 🙏

@pascalgn pascalgn closed this Sep 29, 2020
@michaelbeaumont
Copy link
Contributor Author

michaelbeaumont commented Sep 29, 2020

@pascalgn I addressed all of your comments, this PR works we've been using it for weeks.

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

2 participants