I’ve opened a few tickets in the past, intending for some discussion to take place before they can be accepted and started. On multiple occasions, I’ve seen PRs for these unreviewed tickets, or people claiming them eagerly.
This has a few problems:
- Unreviewed tickets might get additional refinement, changing their scope to something the claimant is no longer happy or comfortable to do
- PRs get opened which cannot be reviewed, since they’re aiming to fix a goal which hasn’t been finalised
- Draft proposed solutions are opened as PRs without much extra review, leading to yet more low-quality submissions (already a problem)
- It can feel like it adds unnecessary time pressure to getting a ticket accepted, since someone is already waiting on direction
- If someone claims an unreviewed ticket, at what is that claim considered stale? Is it relative to their claim date or the acceptance date?
Personally, I’ve always assumed that unreviewed tickets should remain unclaimed and unstarted until they’re accepted. The contributor guide doesn’t explicitly say either way. It does say that the author of a ticket can claim and start an unreviewed ticket, which makes sense, but says nothing about anyone else.
The downside I see of this approach is that people could interpret this as the process being to unofficially claim a ticket in a comment (“dibs”), rather than through a formal process. Alternatively, they can CC themselves, and we get a race to see who can claim it first (and some conflict since Trac doesn’t live-update).
I’ve written a few tweaks to the docs to try and make this clearer.
If there’s consensus, I can turn it into a PR.
6 Likes
Thanks for raising this topic! @theorangeone
While we want to discourage “claiming” unreviewed tickets, we don’t want to stifle someone who has a concrete solution of the ticket which could be a potential issue.
My suggestion here would be that the PR’S for the unaccepted tickets should be labelled as “Ready For Check” which indicates its purpose
While your proposed solutions takes clean and simple approach to the solution I realized is people might still open a PR without claiming the ticket, thinking that “first to code wins.”
To fully address the issues you raised in the initial discussion, would it be worth adding a note about Draft PRs? Specifically, making it clear that PRs on unaccepted tickets are ‘at-risk’ and should be treated as proposals rather than final submissions. This might lower the ‘time pressure’ you mentioned where people feel they have to race to get code in before the scope changes.
Also, do we want to take this opportunity to define when a claim on an Accepted ticket becomes stale e.g., 14 days of inactivity?
As a triager, in some cases I like seeing a sketch of the work to help justify a decision. Otherwise I end up saying “Tentatively accepted, let’s see if this is feasible”. In most cases this is probably the reporter’s job, but I wouldn’t rule out help from someone else.
Overall, yes, I’d be glad to add doc tweaks for this. Reviewers are already leaving this feedback in one-off cases in Trac when we see it.
Alternatively, they can CC themselves, and we get a race to see who can claim it first
I think we already have this race, via the owner field. I tend to respect the “dibs” from an owner assignment even if it predates triage. I sometimes overwrite the owner back to the reporter if there was language like “I can submit a PR”, but they just didn’t have their paperwork in order (forgot to update the Owner field). So I think this is a separate problem.
I’m not in favor – if the new norm in the LLM era is that it takes months to get a review, then it’s not fair to ask a contributor to be 10x faster to reply. I think 3 or 4 months is more like it.
Sorry, my reply was clear as mud.
As a triager, in some cases I like seeing a sketch of the work to help justify a decision.
Even so, I like @theorangeone’s proposal to make a more brighline rule that only the ticket author should be putting up speculative PRs before triage.
I wouldn’t rule out help from someone else.
If someone else wants to help, they can post on Trac a link to a PR against their own repo or a branch diff just like you did in this thread!
2 Likes
on the docs tweaks. I’m not convinced most of the eager ticket jumpers are reading that section of the docs, but it certainly can’t hurt (and helps communicate expectations for those who are trying to follow the process).
We could also look at a Trac mod to block ordinary users (other than the reporter) from changing owner on an un-triaged ticket.
I’ve started assigning myself as owner when I open a new ticket. This mostly prevents pre-triage owner sniping, plus avoids a flood of noisy comments from people asking if they can be assigned to work on it. (After it’s approved I either unassign myself to throw it open for takers, or keep it if I had started working on it.)
4 Likes
Thanks for bringing this up @theorangeone.
I support the proposition and I would go even further by limiting PR submission on unaccepted tickets to reporters with a proven familiarity with the contributing process (e.g. at least one previously merged PR) instead of allowing any authors to do so.
This could be handled in a follow up though as unsolicited poor quality work on tickets has really been plaguing us recently and I would very much appreciate if the documentation stated this is not welcome so I could point at it.
2 Likes
Hi all,
Wanted to share some related work that just landed: we have now an automated PR quality check as a GitHub Actions workflow (Added automated quality checks for PRs as a GitHub Actions workflow. · django/django@4593ae9 · GitHub).
It directly addresses the core concern in this thread: PRs against unreviewed/unaccepted tickets are now flagged automatically. Specifically:
- If a PR references a ticket that isn’t in Accepted stage (with assigned status and no resolution), the bot posts an error comment explaining what the contributor needs to do before the PR can be reviewed.
- If no ticket is referenced at all, contributors are told to file one and wait for triage before submitting a PR.
The check also applies a contributor tier: authors with 5+ commits in the past 3 years are skipped entirely (established contributors), those with 1-4 commits get the comment but the PR stays open, and brand-new contributors with no recent commits get the PR auto-closed on failures.
What is lacking now is more explicit wording on the docs. But with the recently landed harness to write python checks and being able to test them, plus the ability to run them as GH workflows, we could do 
fancier stuff.
Any improvement on that end is definitely welcomed!
4 Likes