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

[DO NOT MERGE] test PR: changes in vNext but not vCurrent, in a forked PR #3111

Conversation

pepopowitz
Copy link
Collaborator

Description

This PR exists as a test case, and will be deleted after #3091 is completed.

Scenario: changes are made in vNext but not vCurrent, in a forked PR.

Expected result: a comment suggesting changes in vNext be ported to vCurrent.

When should this change go live?

Never!

@pepopowitz pepopowitz added the hold This issue is parked, do not merge. label Dec 20, 2023
@pepopowitz pepopowitz self-assigned this Dec 20, 2023
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@pepopowitz
Copy link
Collaborator Author

Ok, this just doesn't work in a forked PR.

The checkout action, because it's running on pull_request_target, is checking out the host's SHA, which is what this is merging into. So it's not running the action with the changes in this PR.

See this conversation for a long discussion of what might be the right thing to check out on pull_request_target.

For now I'm going to revert #3091 to triggering on pull_request instead of pull_request_target, and exclude forked PRs from my acceptance criteria. I'll try to fix forked PRs in a follow-up.

@pepopowitz pepopowitz closed this Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hold This issue is parked, do not merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants