Skip to content

Allow bench workflow to run on PRs from forks#656

Closed
ashkrisk wants to merge 2 commits into
mainfrom
fork-workflow
Closed

Allow bench workflow to run on PRs from forks#656
ashkrisk wants to merge 2 commits into
mainfrom
fork-workflow

Conversation

@ashkrisk

@ashkrisk ashkrisk commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions

github-actions Bot commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Before you submit for review:

  • Does your PR follow guidelines from CONTRIBUTIONS.md?
  • Did you summarize what this PR does clearly and concisely?
  • Did you include performance data for changes which may be performance impacting?
  • Did you include useful docs for any user-facing changes or features?
  • Did you include useful javadocs for developer oriented changes, explaining new concepts or key changes?
  • Did you trigger and review regression testing results against the base branch via Run Bench Main?
  • Did you adhere to the code formatting guidelines (TBD)
  • Did you group your changes for easy review, providing meaningful descriptions for each commit?
  • Did you ensure that all files contain the correct copyright header?

If you did not complete any of these, then please explain below.

# Default branches based on event type
if [[ "${{ github.event_name }}" == "pull_request" ]]; then
echo "Pull request detected. Using main and PR branch: ${{ github.head_ref }}"
BRANCHES='["main", "${{ github.head_ref }}"]'

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.

This seems to be a change from "branch" to "sha", rather than also adding support for the sha. Don't we need it to work for both?

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.

Yes; but this works for two reasons:

  • Git refs can be either branch names or SHAs.
  • The elif condition just after this handles cases where the workflow is manually dispatched by providing a branch name (or SHA). This conditional branch only deals with PRs, and that gives us the freedom to pick how we want to refer to the head commit, either by branch or SHA

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.

In the latest PR (#688) I've changed this to use the Github ref instead of the SHA

@ashkrisk

Copy link
Copy Markdown
Contributor Author

Closing in favour of #688

@ashkrisk ashkrisk closed this Jun 25, 2026
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.

2 participants