Repository navigation
[DISCUSSION] PR Review Process / States / "How to find the PRs in the list that need review" #21038
Description
This is related to the broader discussion about organizing DataFusion's growth:
As the number of open PRs grows, it is getting harder to quickly see which PRs need attention and how best to help.
For me personally, I struggle wiht:
- If I have time to review PRs, it is not obvious which open PRs are waiting on review and which are waiting to incorporate feedback
- If I have time to merge PRs, it is not obvious which already approved PRs are waiting for the 24 hour period .
For example, I personally use these two searches to find waiting PRs
- DF Approved PRs - currently has 21 open
- DF Review PRs - currently has 186 open
I think the core issue is that the current way to find PRs mixes together many different states, which makes it harder to triage efficiently
Also, there is currently no documented "review priority".
Describe the solution you'd like
I would like a clearer and more explicit way to find PRs that need community attention.
I think at the core we probably need some sort of triage process and "workflow" for PR review / merge attention. I am acutely aware that we have very little extra capacity for such a process with the existing committers, but I do wonder if having it might attract others who can help the reviews along
Describe alternatives you've considered
For example, a PR priority definition might look like (not saying this is the right one, it is just a strawman)
- Regression Bugs (what used to work doesn't anymore)
- Bugs
- Performance Improvements
- New Features
For example, a PR review flow might look like this
flowchart LR
A[Needs Initial Review] --> B[Feedback Received]
B --> C[Feedback Addressed]
C --> B
C --> D[Approved, waiting]
D --> E[Approved Ready to Merge]
Activity
alamb commentedon Mar 18, 2026
I think @neilconway may have some thoughts on this one too
adp2201 commentedon Mar 18, 2026
My recommendation would be to document the PR flow first, and only add labels later if there is still a real gap.
A lightweight documented flow like this would already help a lot:
By “define the corresponding queries / saved searches”, I mean mapping each of those states to a GitHub PR search so people can quickly see the current list for that state without adding more process immediately.
For example:
Needs initial review→ open, non-draft PRs that are not yet approvedWaiting on author→ PRs with requested changes / unresolved review feedbackReady for re-review→ PRs updated after review feedbackApproved, waiting 24h→ approved PRs that are still inside the waiting windowReady to merge→ approved PRs past the waiting windowI’d try that first using GitHub’s existing signals before adding labels. Labels can help, but they also create maintenance overhead, and stale labels can make things less clear rather than more clear.
So my preference would be:
alamb commentedon Mar 19, 2026
Thanks @adp2201 -- can you provide an example github search for the states you propose (if they exist it would be great). For example, what would be github PRs for these two states (we can use them today!)
adp2201 commentedon Mar 19, 2026
After digging into it a bit more, I don’t think it is straightforward to distinguish those two states accurately with GitHub search alone.
The closest useful filter for the broad “approved PRs” bucket seems to be:
is:pr is:open repo:apache/datafusion review:approved draft:falseI think that works reasonably well for “approved PRs”, but not for splitting:
Approved, waiting 24hReady to mergebecause the missing piece is the approval timestamp. GitHub search gives us
review:approvedandupdated:, but not “approved at”, which is what we would actually need.So I think the realistic options are:
Use a single approved bucket
Use a manual label/state for the 24h distinction
Use small automation
My current preference would be:
alamb commentedon Mar 21, 2026
I don't know what you are trying to propose (this sounds LLM generated). When you say "bucket" how would that be represented?
kumarUjjawal commentedon Mar 22, 2026
Can we do label based approach, for example, every new PR will bel labeled
needs reviewand whenever a committer(I'm not counting community member reviews here because they don't have the ability to change labels as far as I know) reviews the PR they can remove theneeds reviewlabel and add eitherneeds changes,needs feedbackorapproved. Apart from theapprovedthe other two will remain in that state until the committer is satisfied with the changes and then can lable itapproved. My reasoning for the label approach was purely based on the ability to know state and fileter easily.adp2201 commentedon Mar 22, 2026
@alamb "bucket" is what I am referring to for all PRs that are captured with this filter :
is:pr is:open repo:apache/datafusion review:approved draft:false. Which is basically, single category that tracks all approved PRs and doesn't distinguish between PRs outside and inside of 24 hours boundary. Reviewers will need to manually check what PRs have passed 24 hours time post PR approval. Let me know if this clarifies..@kumarUjjawal the challenge with manual labels is that it adds new process for everything to remember and execute. Whereas, with preconfigured filters, it's much more automatic.
kumarUjjawal commentedon Mar 22, 2026
How does this translates to:
Your suggested filter doesn't have anything related to time.
in my opinion following predefined process is much more manageable.
alamb commentedon Mar 23, 2026
I agree that adding some (simple) labels at first would likely be the most effective here
alamb commentedon Aug 2, 2026
It seems the problem has gotten worse in the last 6 months
alamb commentedon Aug 2, 2026
An update here from me:
Some bright spots:
alamb commentedon Aug 2, 2026
I got inspired and tried to distill my PR best practices into documentation
Docs: Add PR review guide (apache#24051)
Docs: Add PR review guide (apache#24051)
alamb commentedon Aug 17, 2026
Related discussion