boa-dev / criterion-compare-action

⚡️📊 Compare the performance of Rust project branches
ISC License
63 stars 26 forks source link

Linked commit has the wrong target #115

Open sdankel opened 9 months ago

sdankel commented 9 months ago

The linked commit in the action's comment is incorrect. It links to a commit that says "This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository."

image

Instead, it should be linking to the commit that triggered the workflow run, which is part of the PR branch.

See this comment: https://github.com/FuelLabs/sway/pull/5546#issuecomment-1933548947

Example of the action's comment: https://github.com/FuelLabs/sway/pull/5546#issuecomment-1928304573

Which links to: https://github.com/FuelLabs/sway/commit/c1daf98ba4e34562a59150a3c654bcb228307463

sdankel commented 9 months ago

This is very weird, since I see the action is only using the sha from github.context

sdankel commented 9 months ago

https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request

When the event is pull_request, sha is a commit from the merge branch: refs/pull/PULL_REQUEST_NUMBER/merge rather than the PR branch that users see.

When the the event is push, it's the sha that we'd expect. However, branchName: ${{ github.base_ref }} only works with event pull_request.

If the event type is pull_request, the correct commit to use is event.after

  "event_name": "pull_request",
  "event": {
    "action": "synchronize",
    "after": "0cd4b3303ff2bd7987b99570d8a04bcd204a8e76",
    "before": "bc4b6ad9d03ee5652f9e7e917929e3eb18f593f5",