finding PRs from forks

[bryangingechen]

Currently for pull_request / pull_request_target workflows running on PRs coming from a fork, the suggestion of adding sha: ${{ github.event.pull_request.head.sha }} to the config doesn't work, since the workflow searches for the SHA in the commits of the base repository with https://api.github.com/repos/<base.repo.full_name>/commits/<head.sha>.

However, the commit only lives in the forked repository. Maybe a quick way to fix this would be to include a config option which allows users to specify the repo to search, then we could input somethiing like github.event.pull_request.head.repo.full_name and the API call could use that instead. (Happy to make a PR at some point if this sounds reasonable.)

[8BitJonny]

Do you already have an idea on how to achieve this? Because right now this whole library is based on this API endpoint of github: https://docs.github.com/en/rest/commits/commits?apiVersion=2022-11-28#list-pull-requests-associated-with-a-commit

And it seems like for PRs from forked Repos it never finds the SHA as you already said. But I don't think that we can then just call this API endpoint, passing in the forked repo name, and will then find the PR that is open from the forked repo to the parent repo.
Or do you have a different idea?

[bryangingechen]

I think it does work, here's a link using that API endpoint to the commit corresponding to #23 which is currently on a fork of this repo: https://api.github.com/repos/bmalinconico/gh-get-current-pr/commits/c5137dadea613f75e175f52f68cacc4bb3609d94/pulls

For me at least it returns a JSON object with the data of the pull request.

[8BitJonny]

You are totally correct 💯 and this should be fixed.
Maybe even in such a way that this doesn't require an extra config input that says "please look for this sha in repo fork X".

I'm pretty sure we already have access to pull_request.pull_request.head.repo (pull_request type) which should in this case point to the fork.

In any case If you want to make a PR for this, I would be very happy otherwise I'll see when I have time

[bryangingechen]

Thanks! I can try to take a look in a week or two, but probably not before. I'll let you know here before I start working on it, so there's no duplication of effort.

[bryangingechen]

I'm starting to look at this, and I'm thinking of just adding some logic in get-prs-associated-with-commit.ts that checks if github.event_name is pull_request or pull_request_target and then using pull_request.pull_request.head.repo if so. Does that sound reasonable?

If so, I could also do something similar in get-inputs.ts to use github.event.pull_request.head.sha for sha for pull_request* events, which might make the action even easier to use for those cases. (Maybe that should be a separate PR? Let me know what you prefer.)

[8BitJonny]

I'm starting to look at this, and I'm thinking of just adding some logic in get-prs-associated-with-commit.ts that checks if github.event_name is pull_request or pull_request_target and then using pull_request.pull_request.head.repo if so. Does that sound reasonable?

Yeah, that sounds perfect.

If so, I could also do something similar in get-inputs.ts to use github.event.pull_request.head.sha for sha for pull_request* events, which might make the action even easier to use for those cases. (Maybe that should be a separate PR? Let me know what you prefer.)

I don't think this is necessary. Each time the action gets called, it gets called for the latest commit so when you don't specify a sha to the config of this action specifically, it already defaults to github.event.pull_request.head.sha.

[bryangingechen]

I don't think this is necessary. Each time the action gets called, it gets called for the latest commit so when you don't specify a sha to the config of this action specifically, it already defaults to github.event.pull_request.head.sha.

Ah, I was under the impression that users had to supply that explicitly based on this text:

To always find and pass the correct commit SHA to this action use this workflow config:

    steps:
      - uses: 8BitJonny/gh-get-current-pr@3.0.0
        id: PR
        with:
          sha: ${{ github.event.pull_request.head.sha }}

This will then work no matter the trigger event and no matter if it is the first PR commit or not.

But maybe I'm misunderstanding something.

[8BitJonny]

true, my bad. You are right. For the trigger "pull_request" it uses by default the sha of the special merge commit that github creates. But yeah, changing that default behaviour would belong in a separate PR since with merging this PR, the library gets not a feature but it gets a breaking change which will require a bump in major version.

[bryangingechen]

To summarize what I learned from #328: it turns out that fixing this effectively requires the breaking change to the defaults I proposed (opened as #329). This is because we need to pass the non-merge-commit sha to listPullRequestsAssociatedWithCommit, and doing that for "pull_request" and "pull_request_target" triggers while also respecting user-provided overrides is exactly what I do in #329.

Once #329 is in, I'll open another PR for this issue. Apologies for the notification spam!

[8BitJonny]

But does it really need to be based on #329 ? See my comment

[bryangingechen]

In case you missed it, I've reopened #328 based on your comment.

[8BitJonny]

Sorry, I've been just super busy. I'll try and have a look in the next coming days

[bryangingechen]

Hello, do you think you'll have time to look at this in the near future? If not, no worries; I can move to a fork of this action for the time being.