Skip to content

fix(ci): stop pr-review-poster from spamming REQUEST_CHANGES on every push#27055

Merged
mrpollo merged 1 commit intomainfrom
mrpollo/fix-review-poster-spam
Apr 11, 2026
Merged

fix(ci): stop pr-review-poster from spamming REQUEST_CHANGES on every push#27055
mrpollo merged 1 commit intomainfrom
mrpollo/fix-review-poster-spam

Conversation

@mrpollo
Copy link
Copy Markdown
Contributor

@mrpollo mrpollo commented Apr 10, 2026

Branch protection rules block the GITHUB_TOKEN from dismissing reviews (HTTP 403: "Branch protections do not permit dismissing this review"), so every push to a PR with clang-tidy findings added another undismissable REQUEST_CHANGES review. PR #27004 accumulated 12 identical blocking reviews.

Two changes:

Switch to COMMENT-only reviews (pr-review-poster.py, pr-review-poster.yml, clang-tidy.yml):

  • ACCEPTED_EVENTS now only allows COMMENT, rejecting REQUEST_CHANGES at validation time
  • Producer (clang-tidy.yml) passes --event COMMENT instead of --event REQUEST_CHANGES
  • Dismiss logic skips COMMENTED reviews (non-blocking, no dismissal needed)
  • dismiss_stale_reviews now returns a count of failed dismissals and logs it, instead of silently swallowing errors

Findings still show inline on the "Files changed" tab. The CI check status (pass/fail) is what actually gates merging.

Exclude test files from clang-tidy-diff (clang-tidy.yml):

  • clang-tidy-diff-18.py runs on the raw git diff, not compile_commands.json
  • Test files (gated by BUILD_TESTING=OFF) appear in the diff but gtest headers aren't available, producing false positives like gtest/gtest.h not found (seen on fix(navigator): rtl compute wind angle to select best land approach based on rally point location instead of home location #27004)
  • Fix: git diff ... -- ':!*/test/*' filters test paths from the diff before piping to clang-tidy-diff
  • run-clang-tidy-pr.py (the non-artifact path) already handles this correctly by checking against compile_commands.json
  • Enabling CMAKE_TESTING=ON was considered but fails in the clang build (abseil can't find pthreads with the clang-only toolchain in the container)

@mrpollo mrpollo force-pushed the mrpollo/fix-review-poster-spam branch from b9d2ad5 to 094238d Compare April 10, 2026 20:59
@mrpollo mrpollo marked this pull request as draft April 11, 2026 01:52
… push

Branch protection rules block the GITHUB_TOKEN from dismissing reviews
(HTTP 403), so every push added another undismissable REQUEST_CHANGES
review. PR #27004 accumulated 12 identical blocking reviews.

Switch to COMMENT-only reviews. Findings still show inline on the diff
but don't create blocking reviews that require manual maintainer
dismissal. The CI check status (pass/fail) gates merging, not the
review state.

Also enable CMAKE_TESTING=ON in the clang-tidy build so test files get
proper include paths in compile_commands.json. Without this,
clang-tidy-diff runs on test files from the PR diff but can't resolve
gtest headers, producing false positives.

Fixes #27004

Signed-off-by: Ramon Roche <mrpollo@gmail.com>
@mrpollo mrpollo force-pushed the mrpollo/fix-review-poster-spam branch 2 times, most recently from 81588ad to d9a2d7e Compare April 11, 2026 01:56
@mrpollo mrpollo force-pushed the mrpollo/fix-review-poster-spam branch from 6ecc6d1 to ce0a078 Compare April 11, 2026 02:16
@PX4 PX4 deleted a comment from github-actions bot Apr 11, 2026
@PX4 PX4 deleted a comment from github-actions bot Apr 11, 2026
@PX4 PX4 deleted a comment from github-actions bot Apr 11, 2026
@mrpollo mrpollo marked this pull request as ready for review April 11, 2026 02:33
@mrpollo mrpollo merged commit c515f81 into main Apr 11, 2026
75 checks passed
@mrpollo mrpollo deleted the mrpollo/fix-review-poster-spam branch April 11, 2026 02:33
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.

1 participant