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

Why does the number input have a lower priority than the pull request event number? #608

Open
rossjrw opened this issue Apr 17, 2022 · 4 comments

Comments

@rossjrw
Copy link

rossjrw commented Apr 17, 2022

Docs state that the number input has a lower priority than the implicit ${{ github.event.number }} from the context of a pull_request (et al.) event:

### `number`
**Optional** Pull request number for push event. Note that this has a **lower priority** than the number of a pull_request event.

export const pullRequestNumber =
context?.payload?.pull_request?.number ||
+core.getInput("number", {required: false})

Why is this the case? If the end user explicitly specifies a PR number, even during a pull_request workflow, doesn't it make more sense to assume the user knows what they're doing and respect that setting?

@marocchino
Copy link
Owner

In short, for historical reasons.
By the way, your question seems reasonable. Should I edit or would you like to make PR?

@rossjrw
Copy link
Author

rossjrw commented Apr 18, 2022

I don't know what those historical reasons are, so I'll let you edit.

@marocchino marocchino mentioned this issue Nov 29, 2022
@vaab
Copy link

vaab commented Jan 16, 2023

For what it's worth I'd +1 the pr_number which is more explicit than number and wouldn't break backward compatibility.

FYI, I'm working with workflow_run workflows triggered via pull_request workflows to handle comments to be written with all the protection rules. Only the workflow_run has write access to PR comments, and at the same time doesn't have any pr number attached to its context. We have to basically use this example from the doc to manage to send the pr number information through artifacts. And we need to specify the pr number with number input with this plugin if we want it to work.

I had to double-check that number was indeed the way to specify the pr number, and was a little worried by the documentation of this input stating that this might not be the highest priority. And fell here. I understand that all should work, but would like to second the change proposed.

@mgeier
Copy link

mgeier commented Oct 13, 2024

Only the workflow_run has write access to PR comments, and at the same time doesn't have any pr number attached to its context. We have to basically use this example from the doc to manage to send the pr number information through artifacts.

I have found that I can get the PR number in a workflow_run like this:

      - name: Obtain PR number
        run: >
          echo '${{ github.event.workflow_run.referenced_workflows[0].ref }}'
          | sed 's|^refs/pull/\([^/]*\).*|PR_NUMBER=\1|'
          >> $GITHUB_ENV

I can then use it as number like this:

      - name: Create PR comment
        uses: marocchino/sticky-pull-request-comment@v2
        with:
          number: ${{ env.PR_NUMBER }}
          message: |
            ...

This works for me right now, but I don't know if that's officially supported by GitHub.

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

No branches or pull requests

5 participants
@marocchino @vaab @mgeier @rossjrw and others