Skip to content

ci: remove PR title check comment when check passes#850

Merged
bestbeforetoday merged 1 commit into
substrait-io:mainfrom
nielspardon:ci/pr-title-remove-comment-on-pass
Jun 17, 2026
Merged

ci: remove PR title check comment when check passes#850
bestbeforetoday merged 1 commit into
substrait-io:mainfrom
nielspardon:ci/pr-title-remove-comment-on-pass

Conversation

@nielspardon

Copy link
Copy Markdown
Member

Summary

The PR Title Check workflow posts an ACTION NEEDED comment when the PR title/description doesn't follow Conventional Commits, but it never removed that comment once the title was fixed and the check passed. This updates the workflow so the comment is deleted when the check passes.

Changes

  • Collapse the previous failure()-only logic into a single github-script step that branches on the commitlint outcome (continue-on-error: true + core.setFailed(...) keeps the check red on failure).
  • When the check passes, delete any previously posted comment; when it fails, post the comment (if not already present).
  • Identify the bot comment via a hidden <!-- pr-title-check --> marker rather than exact body matching, so detection/cleanup survives wording or whitespace changes.
  • Declare permissions: pull-requests: write so the workflow can delete comments.

Notes

Cleanup is marker-based, so it applies to comments posted by the new version going forward; pre-existing comments from the old workflow won't be auto-removed.

🤖 Generated with Claude Code

bestbeforetoday pushed a commit to substrait-io/substrait-python that referenced this pull request Jun 16, 2026
Ports substrait-io/substrait-java#850 to
substrait-python.

Previously the PR Title Check workflow posted an "ACTION NEEDED" comment
when the PR
title/description did not follow Conventional Commits, but never removed
it once the title was
fixed.

This consolidates the comment logic into a single `github-script` step
that branches on the
commitlint outcome (using `continue-on-error: true` plus
`core.setFailed(...)` to keep the check
red on failure):

- On pass: delete any previously posted comment.
- On fail: post the comment if not already present.

The bot comment is identified via a hidden `<!-- pr-title-check -->`
marker rather than exact body
matching, so cleanup survives wording or whitespace changes. The
workflow now declares
`permissions: pull-requests: write` so it can delete comments.

The Python-specific `.commitlintrc.js` content (the dependabot `Bumps
[...]` ignore rule) is
preserved.

Note: cleanup is marker-based, so it only applies to comments posted by
this new version going
forward. Pre-existing comments from the old workflow version won't be
automatically removed.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Refactor the PR title check into a single github-script step that branches
on the commitlint outcome. When the check passes, any previously posted
comment is deleted; when it fails, the comment is posted (if not already
present) and the job fails.

The bot comment now carries a hidden marker so it can be found regardless
of wording or whitespace, and the workflow declares pull-requests: write
so it can delete comments.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@nielspardon nielspardon force-pushed the ci/pr-title-remove-comment-on-pass branch from 7854bd2 to 16840d9 Compare June 17, 2026 06:43
@nielspardon

Copy link
Copy Markdown
Member Author

rebased on latest main and fixed the conflicts

@bestbeforetoday bestbeforetoday left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Normally I would want to see elevated permissions defined only for the job that need those permissions but this workflow has only one job so permissions at the top level seem OK.

A valid question was raised in the substrait-python repository about whether we need the comment at all, particularly as we then want to go to the trouble of removing it. That can be considered for the future though. This PR seems to add value by removing the visual clutter of a comment that is no longer applicable.

@bestbeforetoday bestbeforetoday merged commit f927c58 into substrait-io:main Jun 17, 2026
12 checks passed
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