Skip to content

Conversation

@rjarry
Copy link
Collaborator

@rjarry rjarry commented Oct 24, 2025

Remove the apply workflow that rebased and merged pull requests in one step. Replace it with a workflow that only amends commits with trailers when reviews are approved or when trailer comments are posted.

The new add-trailer workflow triggers on issue comments containing Acked-by or Tested-by trailers and on successful completion of the Review workflow (which runs when a pull request review is approved). It extracts or constructs the appropriate trailer, rebases all commits in the PR branch to append it, and force-pushes back to the PR branch.

The force push is performed by a dedicated bot account with maintainer permissions. When the bot pushes, most CI checks are skipped to avoid redundant test runs.

The actual merge is now done using the standard GitHub merge button instead of being automated, allowing maintainers to decide when to merge after trailers have been added.

Summary by CodeRabbit

  • New Features

    • Automated trailer insertion into pull request commits when sign-offs are provided
    • New workflow that responds to approved pull request reviews and records review events
  • Chores

    • Removed legacy manual PR application workflow
    • CI jobs updated to conditionally skip certain builds when triggered by an automated bot

@coderabbitai
Copy link

coderabbitai bot commented Oct 24, 2025

📝 Walkthrough

Walkthrough

Adds a new "Add Trailer" workflow to append Acked-by/Tested-by trailers from issue comments or workflow_run events. Removes the "Apply PR" workflow and its .github/workflows/apply.sh script. Adds a new "Review" workflow that triggers on pull request review approvals. Updates check.yml and package.yml to skip specific jobs when the triggering actor is grout-bot.

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "github: split trailer addition from pull request merging" directly and accurately reflects the primary objective of the changeset. The PR removes the combined apply workflow (apply.yml and apply.sh) that previously handled both rebasing and merging in a single step, and replaces it with separate workflows: the new add-trailer workflow adds trailers to commits, while merging is now manual. The title is specific, concise, and clearly conveys the main architectural change without vague language or extraneous details.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 71c18dc and 7c02267.

📒 Files selected for processing (6)
  • .github/workflows/add-trailer.yml (1 hunks)
  • .github/workflows/apply.sh (0 hunks)
  • .github/workflows/apply.yml (0 hunks)
  • .github/workflows/check.yml (3 hunks)
  • .github/workflows/package.yml (2 hunks)
  • .github/workflows/review.yml (1 hunks)
💤 Files with no reviewable changes (2)
  • .github/workflows/apply.yml
  • .github/workflows/apply.sh
🚧 Files skipped from review as they are similar to previous changes (2)
  • .github/workflows/package.yml
  • .github/workflows/check.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
  • GitHub Check: build-and-tests (clang-16, none, debugoptimized, ubuntu-24.04, false)
  • GitHub Check: build-and-tests (gcc-13, none, debugoptimized, ubuntu-24.04, -Dfrr=enabled, false)
  • GitHub Check: build-and-tests (gcc-14, address, debug, ubuntu-24.04, -Dfrr=enabled, true)
  • GitHub Check: build-and-tests (clang-15, none, debugoptimized, ubuntu-22.04, false)
  • GitHub Check: build-and-tests (clang-18, none, debugoptimized, ubuntu-24.04, false)
  • GitHub Check: deb
  • GitHub Check: rpm
🔇 Additional comments (4)
.github/workflows/review.yml (1)

1-19: Looks good.

This workflow serves as a clean trigger point for the add-trailer.yml workflow when reviews are approved. Permissions are appropriately minimal.

.github/workflows/add-trailer.yml (3)

20-23: Multi-line if condition is correct.

The use of >- (folded scalar with strip trailing whitespace) properly handles line folding for GitHub Actions expressions.


26-77: Trailer extraction and PR lookup logic is sound.

The JavaScript step correctly handles both trigger types: parsing trailers from comments and fetching reviewer info from workflow_run events. Email fallback to UNKNOWN_EMAIL is handled downstream.


96-114: Email resolution with fallback is well-designed.

Multiple strategies (git log, git shortlog) to infer missing emails, with proper error handling. The sed escape on line 113 correctly handles angle bracket delimiters.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 119b782 and 71c18dc.

📒 Files selected for processing (6)
  • .github/workflows/add-trailer.yml (1 hunks)
  • .github/workflows/apply.sh (0 hunks)
  • .github/workflows/apply.yml (0 hunks)
  • .github/workflows/check.yml (3 hunks)
  • .github/workflows/package.yml (2 hunks)
  • .github/workflows/review.yml (1 hunks)
💤 Files with no reviewable changes (2)
  • .github/workflows/apply.sh
  • .github/workflows/apply.yml
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/add-trailer.yml

20-20: if: condition "${{ (github.event_name == 'issue_comment' && github.event.issue.pull_request\n && (contains(github.event.comment.body, 'Acked-by:') || contains(github.event.comment.body, 'Tested-by:')))\n|| (github.event_name == 'workflow_run' && github.event.workflow_run.conclusion == 'success') }}\n" is always evaluated to true because extra characters are around ${{ }}

(if-cond)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
  • GitHub Check: build-and-tests (clang-15, none, debugoptimized, ubuntu-22.04, false)
  • GitHub Check: build-and-tests (clang-16, none, debugoptimized, ubuntu-24.04, false)
  • GitHub Check: build-and-tests (gcc-13, none, debugoptimized, ubuntu-24.04, -Dfrr=enabled, false)
  • GitHub Check: build-and-tests (clang-18, none, debugoptimized, ubuntu-24.04, false)
  • GitHub Check: build-and-tests (gcc-14, address, debug, ubuntu-24.04, -Dfrr=enabled, true)
  • GitHub Check: build-cross-aarch64
  • GitHub Check: deb
  • GitHub Check: rpm
🔇 Additional comments (6)
.github/workflows/check.yml (1)

21-21: Guard pattern is sound. Skipping resource-intensive jobs when grout-bot commits prevents CI waste during trailer amendments.

Also applies to: 112-112, 152-152

.github/workflows/review.yml (1)

1-19: Minimal but functional. Serves as trigger for add-trailer workflow on approved reviews.

.github/workflows/package.yml (1)

17-17: Consistent with check.yml guards. Skips packaging when grout-bot commits trailers.

Also applies to: 67-67

.github/workflows/add-trailer.yml (3)

96-114: Solid email resolution fallback. Gracefully handles missing emails by searching git history and shortlog, with clear error handling.


33-70: Trailer extraction logic is sound. Regex correctly parses acked/tested-by formats, and Reviewed-by construction from workflow actor is appropriate. UNKNOWN_EMAIL placeholder is resolved downstream.


79-85: Token-based checkout with correct persistence. Using TRAILER_BOT_TOKEN for push permissions and persist-credentials: true to retain auth for the force-push step.

Remove the apply workflow that rebased and merged pull requests in one
step. Replace it with a workflow that only amends commits with trailers
when reviews are approved or when trailer comments are posted.

The new add-trailer workflow triggers on issue comments containing
Acked-by or Tested-by trailers and on successful completion of the
Review workflow (which runs when a pull request review is approved).
It extracts or constructs the appropriate trailer, rebases all commits
in the PR branch to append it, and force-pushes back to the PR branch.

The force push is performed by a dedicated bot account with maintainer
permissions. When the bot pushes, most CI checks are skipped to avoid
redundant test runs.

The actual merge is now done using the standard GitHub merge button
instead of being automated, allowing maintainers to decide when to
merge after trailers have been added.

Signed-off-by: Robin Jarry <[email protected]>
@christophefontaine christophefontaine merged commit a478950 into DPDK:main Oct 24, 2025
12 checks passed
@rjarry rjarry deleted the apply-trailers branch October 24, 2025 09:28
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