refactor: determine PR number/sha using GitHub API in report.yml #4361
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughWorkflow steps in Changes
Sequence Diagram(s)sequenceDiagram
participant Workflow
participant GitHub CLI
participant GitHub API
Workflow->>GitHub CLI: Query PR number and SHA (gh pr view)
GitHub CLI->>GitHub API: Request PR info (repo, branch)
GitHub API-->>GitHub CLI: Respond with PR number and SHA
GitHub CLI-->>Workflow: Set environment variables (PR_NUMBER, SHA)
Workflow->>Workflow: Conditionally run steps if PR number found
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
.github/workflows/report.yml (1)
81-92: Undefined,PR_SHAis – comment empty it will render.Inside the Render comment step the script expands
$PR_SHA, yet this variable never enters the environment. Outputs written to$GITHUB_OUTPUTdo not propagate to the shell session. The result: the SHA placeholder disappears, confusing reviewers.Patch like this, you should:
- - name: Render comment - if: steps.get-pr-number.outcome == 'success' - run: | + - name: Render comment + if: steps.get-pr-number.outcome == 'success' + env: + PR_SHA: ${{ steps.get-pr-number.outputs.PR_SHA }} + run: |(or reference it via
${{ steps.get-pr-number.outputs.PR_SHA }}directly inside the YAML).Do likewise for any later steps that rely on the same value.
🧹 Nitpick comments (1)
.github/workflows/report.yml (1)
55-69: CLI query clever, still a guard add you might.
gh pr viewfails with non-zero exit when no PR matches; the whole job then dies. If graceful degradation you prefer, wrap the command:if ! gh pr view … ; then echo "No matching PR found – skipping comment"; exit 0 fiSmall, this is, yet spares red crosses when the workflow fires on non-PR runs or closed PRs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/builds.yml(1 hunks).github/workflows/report.yml(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (24)
- GitHub Check: merge-sentinel
- GitHub Check: CI Bridge / lcg_107: [alma9, clang19]
- GitHub Check: CI Bridge / lcg_107: [alma9, gcc14]
- GitHub Check: CI Bridge / build_gnn_tensorrt
- GitHub Check: CI Bridge / build_linux_ubuntu
- GitHub Check: CI Bridge / linux_ubuntu_2404_clang19
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc14]
- GitHub Check: CI Bridge / lcg_106a: [alma9, clang16]
- GitHub Check: CI Bridge / build_exatrkx_cpu
- GitHub Check: CI Bridge / lcg_107: [alma9, gcc13]
- GitHub Check: CI Bridge / build_exatrkx
- GitHub Check: CI Bridge / lcg_107: [alma9, gcc13]
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc13]
- GitHub Check: CI Bridge / linux_ubuntu_2204
- GitHub Check: CI Bridge / clang_tidy
- GitHub Check: CI Bridge / clang_tidy
- GitHub Check: build_debug
- GitHub Check: linux_ubuntu_extra (ubuntu2204, 20, g++)
- GitHub Check: macos
- GitHub Check: linux_ubuntu
- GitHub Check: linux_ubuntu_extra (ubuntu2404_clang19, 20, clang++-19)
- GitHub Check: docs
- GitHub Check: missing_includes
- GitHub Check: unused_files
🔇 Additional comments (1)
.github/workflows/builds.yml (1)
211-216: Directory ensured, failure avoided, good this change is.
mkdir -p physmonbefore thecatcommand you add – the “file not found” error, it shall prevent.
a7b96a8 to
16180f1
Compare
|
@veprbl This now uses the gh cli client. This looks like it might work. Does the branch name contain the source repo? Otherwise could there be problems if there are PRs from different forks with the same name? |
You can't make multiple forks (or forks of forks) in the same owner namespace, so "owner:branch" should uniquely identify a PR. |
|
So, turns out it is possible to create multiple forks, but only if owner is an organization. https://github.com/veprbl/acts/actions/runs/15863838553/job/44726751681#step:5:13 |
|
This is probably fine @veprbl. Let's merge this and try it out |
This refactors Physmon report action to query PR number and head sha from GitHub API. The goal here is to get rid of dependency on physmon artifact, so that we are able to post reports from other workflows without having to wait for this artifact to appear.
--- END COMMIT MESSAGE ---
This is a second attempt at #4328. It looks like, indeed, my original solution had an issue with github hiding information about open pull requests. It appears that running
gh pr viewis the accepted solution: https://github.com/orgs/community/discussions/25220#discussioncomment-11316244This was tested to produce a posting for a cross-repo PR: veprbl#6
Summary by CodeRabbit