[Plan] Needs review issue queue management

I think there are lessons to be learned from the Bug Smash Initiative - the format there seems to work well. Even there we don't always move issues on, but the group discussion/triage is helpful and easy for contributors to spend a small amount of time on.

Re #2.1 it would be good to name the channel in this issue so others can find it!

Re #2.2 separate credit should not be needed, given that a good review gets credit on the issue when it is marked fixed. Bug Smash does award credit for closing issues other than fixed, but that should only be a last resort in the "needs review" queue as far as I can see.

Re #2.3 the Bug Smash "random issue of the day" works well - maybe we just need to adopt this for reviews too, possibly with a time limit so we only pick from more recent issues? (these are more likely to have the contributor still around and actually waiting for feedback; with stale issues the original contributors have already left, in some cases)

Thanks, yeah I was planning on doing pretty much what bugsmash started on a different set of issues :)

Just a reminder that How is credit granted for Drupal core issues exists.

I agree with longwave that attention to NR would be best at the front of the queue, for the reasons already mentioned.

What is the plan here for older and stale issues at Needs Review in each category (bug, FR, Plan, Task)? Bus smash has developed strategies and practices for addressing these but I am not aware of that for the other categories.

I think we'll have two phases for this work:

1. One is big clean-up to get as low as possible with the NR queue. First thing there is put back the 1k+ issues in NR back to NW on "technicalities" (patch doesn't apply, commit checks fails) if people are still interested in those issues there'll be some movement and we'll deal with them as the come back in the NR queue.
2. Second one is keeping that NR queue at a manageable level over time, and we should start to try to figure that one out before we do the mass status change on the 1k+ issues that needs work, otherwise we'll end up in the same situation.

For the 1st phase the plan is just to go out and triage issues manually and see what's the state of the queue. For example I was surprised to see the number of "almost RTBC" patches in the queue. There are some that have been RTBC 5 or 10 times already, and not much is left to fix for a legitimate RTBC so we can concentrate on those first. I personally don't have a set plan on how to deal with X or Y issues kinda just waiting to see what happens while I go through a few pages of issues.

In any case this will introduce changes in how we're working overall. I feel the NR queue is the responsibility of core maintainers, just like the RTBC queue is the one of core committers. But it doesn't seem to work that way in real life so maybe we should discuss that a bit as well? who should be responsible for the NR queue? either overall or by component/module.

Most likely all 20 will need to be sent back for rerolls, issue summary, etc. But (I May be wrong) that doesn’t count as a review correct?

I don't assign credit for 'needs re-roll', I do sometimes assign credit for 'this needs an issue summary update' if it's not just tagging - i.e. 'current patch doesn't match the issue summary any more so this needs an update' because that's directly saving me time as a committer having to figure out the same thing, hard to quantify though.

What bugsmash does is have a triage issue where you can post about issues you've triaged, and then you get one issue credit when that issue is marked fixed. There might be less duplicates/outdated in the needs review issue but it won't be zero.

Since bugsmash is focused on bugs regardless of status, is the plan here to focus mostly on features and tasks to avoid overlap, or on 'needs review' regardless of category. I don't think it matters as long as the same issue doesn't come up in different channels at the same time.

There are some interesting data already though:

• For most issues there is an average of 4 automated version change (the bot automatically moving an issue from 10.0.x to 10.1.x) so they tend to stay open for a long time
• For about half the queue more than 50% of the issue status change are to put the issue back to Needs Review
• half of the NR issues are in 25 components:
  

  component	issues	percent of issues failing automated tests
  base system	187	58%
  views.module	169	47%
  entity system	134	50%
  field system	64	55%
  phpunit	59	49%
  other	57	54%
  media system	56	63%
  Claro theme	53	75%
  documentation	50	36%
  forms system	48	56%
  javascript	46	72%
  theme system	45	67%
  node system	43	58%
  layout_builder.module	42	40%
  system.module	40	65%
  configuration system	36	56%
  file system	34	50%
  language system	34	56%
  database system	31	58%
  render system	30	50%
  user.module	30	60%
  ajax system	29	79%
  user system	29	55%
  cache system	28	54%
  comment.module	27	41%
  

I'll try to make some charts at some point.

For example I was surprised to see the number of "almost RTBC" patches in the queue. There are some that have been RTBC 5 or 10 times already, and not much is left to fix for a legitimate RTBC so we can concentrate on those first.

I think this itself is a symptom of the size of the queue, in that you end up with people spread across dozens of active issues that have been running for months or years, and individual issues literally get forgotten about. Also it's usually the person most actively working on an issue who sets it to CNR and then they can't do much from there except for spam people in slack for reviews.

In any case this will introduce changes in how we're working overall. I feel the NR queue is the responsibility of core maintainers, just like the RTBC queue is the one of core committers. But it doesn't seem to work that way in real life so maybe we should discuss that a bit as well?

For anything that's not a module or theme, the components don't really match reality. Me and xjm tried to reorganise this a bit, but ironically that stalled too #2660144: [Plan] Update core components.

Suggestion:
"This issue is being reviewed by the kind folks in Slack, #need-reveiw-queue. We are working to keep the size of Needs Review queue to around 200."

I've been thinking about this for some time and there are few things I keep coming back to though.

1.

I feel the NR queue is the responsibility of core maintainers, just like the RTBC queue is the one of core committers.

The use of 'responsibility' here concerns me. The way this is written, it is open to interpretation and can be read in such a way to mean that the community does not have a responsibility to triage those issues. That once an issue is NR or RTBC a non-maintainer/non-committer has nothing further to do. In practice, we know that is not true. There is even a contributor task to Triage the Drupal core RTBC queue. Nor does the job descriptions for core maintainers that maintainers/committers have this responsibility either.

2. This one is a really a reminder, that triage includes mentoring. It seems worth saying again when the resolution is focused on numbers.

3. A review is not simply marking a patch as needs a reroll. And marking several as such does not meet the existing criteria for credit. Nor does it make sense to me to give credit for something that can be automated.

4. For myself, I think long term progress would be gained by exposing the review process. The new channel could be used for this. There are contributor tasks that lay the process out and could be used as guides. Perhaps, work on an issue all the way to commit, triage one a day, invite people from other channels, etc

Regarding incentives, I think it is best to wait for the outcome #2021489: [Policy] Automatically check whether 'needs review' patches apply and pass linting.

Actively reviewing newer tickets sounds good, we can hopefully get automated linting to clear out the end of the queue anyway and eventually meet in the middle.

I think it would be worth announcing the new channel in #contribute/#bugsmash/#d10readiness

To help keep the momentum of this great initiative and attract more reviewers, perhaps it's worth considering blogging about it? It could be just a short piece every other month, with an update on how many issue got fixed, how the stats are evolving, which new users joined and pledged to review issues, and so on. The blog could get included on Planet Drupal?

Sharing the goals and progress via a blog post would spread the word wider about this vital work. And it is more that statistics, it is an opportunity to discuss each part of the review process, when to ping someone, when not to ping someone, what tags to use, why keeping the issue summary up to date help, how to postpone an issue etc. And maybe there is a sense that reviews or reviewers are improving there skills.

If the blog posts included more that statistics, then the frequency the post would likely be less. At that can be a good thing.

Thanks @smustgrave and @quietone for the feedback, I really appreciate it. Based on it, I created an issue.