andygrove opened a new pull request, #6016:
URL: https://github.com/apache/datafusion-comet/pull/6016
## Which issue does this PR close?
N/A. This adds contributor tooling rather than fixing a filed issue.
## Rationale for this change
We have skills for reviewing a PR's code (`review-comet-pr`,
`deep-review-comet-pr`) but nothing
that checks whether a PR is in a state where reviewing it is worth doing.
Two things come up
repeatedly and both cost reviewer time before any code gets read.
The first is the PR template. A PR that arrives with no rationale, no
description of what changed,
and an empty testing section makes the reviewer reconstruct all of that from
the diff. The template
is not enforced anywhere — CI checks the title format and applies type and
area labels, but nothing
looks at the body.
The second is duplicate work. With ~170 open PRs, two contributors
independently solving the same
problem is common, and the pair usually is not noticed until one of them is
nearly ready to merge.
The recurring shape here is codegen dispatcher versus a native
implementation for the same
expression: same serde object, same `.sql` fixture, mutually exclusive,
closing two different
issues so nothing links them. Catching that early costs one comment;
catching it late costs
somebody's week.
## What changes are included in this PR?
Adds `.ai/skills/pr-triage/SKILL.md`. Given an optional list of PR numbers,
or by default every
open non-draft PR from the last four weeks with bots excluded, it:
1. Reads `.github/pull_request_template.md` and derives the required
sections from that file rather
than hardcoding them, so the skill does not drift when the template
changes.
2. Checks each body for missing headings, headings with nothing under them
once HTML comments are
stripped, and the `Closes #.` placeholder, and verifies any referenced
issue exists and is still
open. A closed issue is treated as a duplicate signal.
3. Looks for competing PRs using IDF-weighted file overlap, then shared
closing issues, then the
diff itself. Hub files like `planner.rs` and `QueryPlanSerde.scala` are
down-weighted out because
dozens of open PRs touch them at once and they carry no signal.
4. Posts at most one comment per PR, written as prose rather than a findings
list, and marks it with
an invisible HTML comment so a later pass does not comment twice.
The skill explicitly does not review code and does not repeat what CI
already checks. It does not
change labels and does not close, approve, or merge anything. Posting is
gated on a single
confirmation for the whole batch, since these are public comments.
## How are these changes tested?
There is nothing executable to unit test here, so I ran the skill's own
analysis steps against the
live repository in read-only mode: 170 open PRs, 63 in scope after
filtering. Two false-positive
sources turned up and both are now written into the skill.
Rust tests live in inline `#[cfg(test)]` modules, so the changed-file list
alone cannot tell you
whether a Rust PR added tests. #5982 touches a single `.rs` file and nothing
else but adds 30 lines
of tests; a file-list check would have wrongly flagged it, so the skill
greps the diff instead.
Separately, `isDraft` is not the whole test for work in progress — several
open PRs are marked
`isDraft: false` but say `[WIP]` or `[draft]` in the title, so the skill
treats the title marker the
same way.
The overlap analysis reproduced pairs I had previously found by hand,
including #5607 and #5874,
which is the worked example in the comment-voice section of the skill.
I have not posted any comment from the skill yet; that is the next thing to
try once this lands.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
