[policy, no patch] Policy to help less interested Patch move forward.

Yeah being a casual contributor to core I don't have the connections to get others to seriously review my work, which sucks. Thanks for proposing something to do about it.

With this approach, I can see it would ensure patches move beyond the dreaded "Needs Review" state of the workflow. But, it would potentially still take a long time. At that point, the only path forward for the patch from there would be for a committer to review it, and I'm concerned that they'd just regard these "self-signed" contributions as a pile of old, possibly no longer relevant, not community reviewed stuff we'd still find our efforts largely or entirely ignored.

What I'd really like to see to help with this problem are two things:

• For new features, a new feature proposal area of d.o. In one of the core conversations @ NOLA @webchick said it really doesn't work ever to do a feature as a lone wolf, due to this problem. Great, we should have a site where you can find others who have vested interest in given features
• For already written patches, esp. bugfixes, a review swap area of d.o. On some of my patches I get to the point where I'd gladly dig into someone else's patch enough to perform a quality review of it, if I'd get them to do the same for me in exchange. A formal place to establish these connections on d.o would be very helpful.

I totally agree that this is a huge problem, but I think this is a cultural problem which cannot be solved with a technical solution. I think we should continue to ensure other contributions than writing patches are equally if not more valuated.

In general I think helping others and build a relationship with folks will help you dramatically to help to get your fixes in. On the other hand I cannot stress out more that I believe maintainers should triage their queues, no matter whether its critical, major or normal.

If the waiting time is passing X period, contributors able to self-signed their patches. We can set a new Review Status to "Self-Signed" or something else. Therefore, anyone is interested, or subsystem maintainers should take care of them.

One thing I'm wondering is, what would "Self-Signed" actually mean. Is this basically another term of RTBC? How would those patches get to the next level afterwards?

But don't forget the 10x developers

In my humble opinion there is nothing like a 10x developer. The people which actually are 10x more productive are the ones which enable 10x more people to get something done.

Yes, that is. We trust committers would make their right decision. If not, we will create another unhealthy section for the community.
"Self-Signed" status & "the X period for the different group of developers" make things more controllable at beginning. Then we can revise its performance to next steps. Ideally, it should reduce the commit path for follow-up issues, we human will make mistakes (even with current RTBC system) but we can fix it instantly.

While this seems to be an improvement for particular users, as you move nearer to the commit, I think this basically just replaces the time people spend in reviewing a patch with time spend by the committers doing the same. This at the end actually scales even less.

Wish Drupal has https://github.com/facebook/mention-bot. (also @see: https://code.facebook.com/posts/597378980427792/)

I agree that something like this could be helpful.

Here is an interesting thing rust is doing, they randomly pick a person to review stuff example. In general all stuff in https://github.com/servo/highfive/tree/master/handlers would be just amazing!

I was thinking how I could help this problem. One way may be to allow this: if the patch provides pass/fail proof of a bug fix? Meaning it it has test coverage as well.

Could that be a middle ground of sorts? Also an "incentive" to write tests which are at times painful to write.

The way to help move the patch forward is to make it easier to review. Update your issue description and make it more clear.

I can say with authority this has been a problem for more than a decade. A casual google search found this post from our esteemed Drupal 8 core release manager from 2007 and I would like to point out this tidbit from it:

More people are writing patches than reviewing them.

This is the hard truth: writing a piece of code is a trivial exercise, writing a test is perhaps a little harder but only because it's more tedious but understanding the potential implications of it in the vast Drupal code ecosystem? What are the future implications, does this change make the code more brittle? Harder to maintain? Now that's a challenge (one that most reviewers actually fail but that's for another time).

What we need here is a mark that "I think this is a change limited to its own namespace" -- the chances of #2800727: Views NumericFilter regular_expression operator wrong method 'op_regex' breaking anything outside of NumericFilter is nil. So that's marked as "I hope this is easy to review". On the other hand, while #2798981: ListNormalizer fails to pass on context variable to field items. is a very small change it actually needs thinking: is this the first time the context is actually passed? If so, are the other classes ready? Etc. So it's not marked because it's bleeds outside of its own namespace.

Now we can direct people to the easy review issues and have the people who actually have an oversight over the whole shebang spend their time on the "bleeding" ones.

Chx, so do you think we should add a new field (to the issue) that helps categorize the scope of an issue? the goal being to help identify how big of a review is needed.

Maybe an issue needs to be checked off (reviewed by) multiple people. For example: A javascript expert, a subsystem maintainer, a documentation reviewer, A QA expert (to examine the tests.)

That'd be the death of the issues, we are struggling to find one person currently.

Probably yes, a namespace one/many selector. It'd be an almost mechanical setting and easy for reviewers to escalate to many if they catch the patch is broader than the writer thought.

How about we have a new block, "Issues for review" that somehow suggests issues to review. Because "Issues for [username]" only shows issues that you've commented on.

Also, if we could somehow label comments as a review, then we could keep a count of reviews. Keeping a count will help generate more reviews.

+1 for a review swap area like was mentioned in #3. I would review more patches if it would lead to more of my patches being reviewed.

It could potentially be one and the same with an "Issues for review" block. For example, say you review an issue that appears in your "Issues for review" block. Then one of your issues could start appearing in other people's "Issues for review" block. To get it seeded and reward prolific reviewers, reviewing 5 issues that don't appear in your block could get one of your issues to start appearing.

What about other incentives? The primary contributor for a patch gets special recognition in their profile. Everybody who posted a patch is automatically selected to receive regular recognition by the automatically generated git commit. The reviewers don't get anything unless the committer explicitly adds them. A new section on the profile pages could be added that tracks how much reviewing a person does. It would likely need the new option to mark a post as a review mentioned before, but it could generate more reviewing.

Although I don't have patches waiting months in review I have peeked at the older issues and wonder about the processes involved that causes this and the person who's work sits waiting. And yet, review is not easy, as chx states in #12.

Of the ideas mentioned above, here is what I prefer. A review swap area (#3, #17) is interesting and something I would participate in. Identifying the scope of the patch (#12) would certainly help potential reviewers find patches that better match their own skills. And definitely, the reviewers needs to be acknowledged (#17).

Finally, a section in the handbook that explicitly explains the actions that need to be taken in a review. For example, there is a novice code contribution guide, but not a novice code review guide. Or, at least, not one that I can find.

@quietone The problem with a novice code review guide or novices reviewing code in general is that novices aren't supposed to set any issue status to RTBC.

The patch has received a thorough review and test by one or more experienced developers, and they have deemed it as ready to be committed to the project.

So what about relaxing this restriction?

The whole section seems to discourage using RTBC in general. Is that necessary?

I've had patches that get reviewed once or twice and then after making the requested updates the reviewer just stops commenting. Perhaps more issues would move forward if that section of the documentation was more encouraging. Getting reviews alone isn't helpful unless it's going to lead to RTBC.

requested updates the reviewer just stops commenting

Yeah, I think most of the people are lazy, its an intrinstic property of humans.

Getting reviews alone isn't helpful unless it's going to lead to RTBC.

Interesting view of the world.

Personally I think the problem is really that there are not enough people reviewing others people's changes.

@jmuzz where are you referencing that quote in #20? That needs to change, RTBC shouldn't need to be by 'experienced developers' in all cases. Some changes don't require developers as they would be suited for writers or designers or other types, and we don't promote that view in core mentoring.

I have to agree with @joelpittet
Noone got fired to set an issue to RTBC ;)

Maybe some of these people are lazy. Maybe some of them don't feel they are in a position to be able to RTBC issues because they are not an experienced developer (emphasis from documentation).

I don't think the problem is that too many people are RTBCing things, so maybe the documentation could be more friendly about it.

I wasn't trying to promote a personal view. I thought that comment was fairly objective. Sometimes an issue gets a review, the developer comes back to work on it more, then it never gets reviewed again or reaches RTBC. We should be able to agree that isn't the result we are hoping for.

@joelpittet The documentation page that appears on is here: https://www.drupal.org/node/156119 .

It's linked to right under the Status dropdown in the issue metadata.

@jmuzz I found it in the Status docs and changed it to remove that, thanks for pointing that out. I've had people think that they had to be 'experienced' and was wondering were they were getting that notion from, now I know.
https://www.drupal.org/issue-queue/status

@jmuzz, I'm not sure I expressed my point well. I am trying to say that there isn't a guide to what actions to take when reviewing. Nor what actions are suitable for what type of patches (typos, complex random fails, documentation, coding standard fixes etc).

@joelpittet, thanks for updating the Status doc.

there are not enough people reviewing

Based on the backlog of NR patches, this is true. Perhaps, at sprints, there could be a mentor and space for people wanting to learn or improve their reviewing skills? At least, the few sprints I have been too this wasn't available. The focus was on fixing things and helping new comers. Maybe things are different at a DruplCon?

Adding related issue

There has not been any discussion here for 7 years. Several proposals have been made which were added to the Issue Summary 2 years ago. There has been no response since then.

Lately a great deal of work has been done, lead by smustgrave to make the size of the 'Needs review' queue manageable, They are doing that by actually doing the reviews, asking questions in various Slack channels and sometimes also finalizing patches. This has resulted in a significant drop of the number of issues with the 'Needs Review' status. And most importantly, most of those issues are being committed and Fixed.

So, is there anything else to do here? Is there anything in the proposals that anyone would like explore or implement?

I asked in #contribute about this. smustgrave, FeyP, andy-blum, dww and cilefen responded. There was no objection to closing this issue.

In the discussion, the topic of self-RTBC was raised. It was pointed out that this is not good practice as it defeats the purpose of code review, which is to get more eyes on the change. And, tt also puts more work on the core committers. Although, at least one experienced contributes will self RTBC when making minor formatting changes.

There were two suggestions that someone may wish to follow up on.

1. For an auto comment for issues tagged 'Needs usability review' that states that there is a regular usability review meeting.
2. An auto triage bot that makes suggestions when a new issue is filed

For the first point, there are currently 142 Open issues that are tagged 'Needs usability review'. I wonder how many of those are stalled because people do not know how to prepare for and join the review?

I am not yet closing this issue because i need to think about how to address the two suggestions (and I am on holiday right now). I will ask in the Slack thread if anyone wants to make issues for those.

Thank you, FeyP, for adding the ideas from the Slack discussion to an existing issue about an automated issue bot. That issue is #3317273: Set up a Drupal.org GitLab issues bot.

Now that is done there is nothing more to do here, so I am closing this as outdated.

Adding parent to help find related issues