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.
4 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!
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.)
2 Likes