Skip to content

CI (upload-dev-build): Correctly get PR number for forks, and exit workflow if PR number or SHA is empty#7806

Open
emilykl wants to merge 2 commits into
masterfrom
fail-if-no-pr-number
Open

CI (upload-dev-build): Correctly get PR number for forks, and exit workflow if PR number or SHA is empty#7806
emilykl wants to merge 2 commits into
masterfrom
fail-if-no-pr-number

Conversation

@emilykl
Copy link
Copy Markdown
Contributor

@emilykl emilykl commented May 20, 2026

Closes #7805

  • Update upload-dev-build workflow:
    • Use gh CLI to get PR number for forks (github.event.workflow_run.pull_requests[0].number does not work for forks)
    • Exit workflow if PR number is empty, rather than proceeding

Steps for testing

The workflow_run trigger scenario (normal case) can't be directly tested until on master due to GitHub Actions security measures. However, the key aspects of this change can be manually verified:

Verifying that the workflow fails if PR number is empty

  • Manually trigger the workflow using the "Run workflow" button on this page, making sure to select this branch (fail-if-no-pr-number from the dropdown)
  • Enter nothing for the PR number, and 26179043520 for Run ID
  • Confirm that the workflow fails
    • Note: When run via manual dispatch, the missing PR number also causes the "get SHA" command to fail, which is why the error message reads "Failed to get commit SHA, exiting"

Verifying the gh CLI command for getting the PR number

  • Run the following command on your local machine in the plotly.js repo root directory:
    gh pr list --search "sha:b7a0c06e08cfa147d235d6a470cde4bfb7549e98" --state open --json number --jq '.[0].number'

@emilykl emilykl requested review from camdecoster and roman-nb May 20, 2026 17:32
camdecoster
camdecoster previously approved these changes May 20, 2026
Copy link
Copy Markdown
Contributor

@camdecoster camdecoster left a comment

Choose a reason for hiding this comment

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

I think you need to update the displayed URL too for the post build message.

Comment thread .github/workflows/upload-dev-build.yml Outdated
echo "Builds for PR #${{ steps.setup-metadata.outputs.PR_NUM }} can be accessed at:" >> $GITHUB_STEP_SUMMARY
echo "- Latest build for this PR: [$BASE/latest/plotly.min.js]($BASE/latest/plotly.min.js)" >> $GITHUB_STEP_SUMMARY
echo "- Build for this commit: [$BASE/${{ steps.setup-metadata.outputs.SHA }}/plotly.min.js]($BASE/${{ steps.setup-metadata.outputs.SHA }}/plotly.min.js)" >> $GITHUB_STEP_SUMMARY
echo "- Build for this commit: [$BASE/${{ steps.setup-metadata.outputs.SHA }}/plotly.min.js]($BASE/${{ steps.setup-metadata.outputs.SHORT_SHA }}/plotly.min.js)" >> $GITHUB_STEP_SUMMARY
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
echo "- Build for this commit: [$BASE/${{ steps.setup-metadata.outputs.SHA }}/plotly.min.js]($BASE/${{ steps.setup-metadata.outputs.SHORT_SHA }}/plotly.min.js)" >> $GITHUB_STEP_SUMMARY
echo "- Build for this commit: [$BASE/${{ steps.setup-metadata.outputs.SHORT_SHA }}/plotly.min.js]($BASE/${{ steps.setup-metadata.outputs.SHORT_SHA }}/plotly.min.js)" >> $GITHUB_STEP_SUMMARY

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch -- updated!

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.

[CI]: Dev build upload proceeds even if PR number is empty

2 participants