Problem/Motivation

There are too many issues that are not actionable by the original issue participants or a Core Maintainer.

You can see the gigantic list at - https://drupal.org/project/issues/drupal?status=8 - when this issue was first opened the count was 2388.

It's near impossible to get a timely review for most issues outside of IRC.

It's not unusual for issues that might only need trivial feedback to be waiting in the "Needs review" list for multiple years.

A large number of stagnant CNR issues represents a lot of wasted volunteer effort.

Active, Postponed and "Needs work" patches usually have some known further action that can be taken by the thread participants to progress the issue, whether it's discussion or code. "Needs review" can be hard for a patch to break through as often a commitment is required from a community member who had no prior engagement with the issue.

Proposed Resolution

Adapt a simplified version of the "PAReview Bonus" Program from the Project Application queue to the Core issue queue to promote and focus the patch review process.

You can read all about the PAReview Bonus project at https://drupal.org/node/1975228.

Proposed core review bonus process:

Choosing an issue:

Important: Only choose an issue that you feel you can do a good job reviewing. Don't feel obliged to be pushed way outside your comfort zone.

We want to prioritise reviewing issues that other people have tagged, so start by trying to pick an issue from the Core review bonus list (you can ignore issues you tagged yourself).

If the bonus list is empty (or you can't find anything there you feel comfortable reviewing), pick any issue you like from the normal "needs review" list.

If there are no issues in the normal review list, come back tomorrow!

Are you new to contributing? Don't worry, that's what the "Novice" tag is for!

Novice, Core Review Bonus issues can be found here: https://drupal.org/project/issues/search/drupal?status%5B%5D=8&issue_tag...

Normal Novice issues that need review can be found here: https://drupal.org/project/issues/search/drupal?status%5B%5D=8&issue_tag...

Performing a good review:

Here's a collection of links to existing docs to help you understand the Core development process.

A review does not have to be long but it should be as thoughtful as possible, within your personal ability.

A good quality review will usually take at least 15 minutes and could take as long as an hour or more if the patch is very large or involves a complex testing process.

The "Core Review Bonus" tag

Once you've left a comment on an issue, if the issue has made some progress (see below) then you've earned a tag and can remove the "Core Review Bonus" tag from the issue you just reviewed (if it's there).

Other people need to be able to see what is going on, so this is a two-step process. Leave a note on or just under your review comment like this:

#2094585: [policy, no patch] Core review bonus for [#12345]

Then go to your issue #12345 and tag it with "Core Review Bonus", with a link back to your review comment at the same time. This way your issue will turn up in the Core Review Bonus issue queue automatically and other people will be able to see why it was tagged.

Because other people participating in the Core Review Bonus are working through tagged issues first, this ensures that the issues you need feedback for are given a little more attention by the community than other issues of the same priority.

"Progress" on an issue could be:

  • The issue has moved to a new status (most often back to "Needs Work" with your feedback)
  • The issue summary has been updated to match the standard template (linked to above), or the documentation for the patch has been improved.
  • Performance profiling has been done
  • Manual testing has been performed and steps to reproduce have been provided in the issue summary for others to verify the testing results

Automated reviews are useful (keep doing them until #1299710: [meta] Automate the coding-standards part of patch review lands!), but people appreciate it most when you take the effort to do a manual review alongside a scripted one. A review that is purely automated and no human has read over the patch or tried to understand the problem that the patch solves, while useful, should not be used as part of the Core Review Bonus - See comment #30 in this thread.

Remaining tasks

  • Review some patches
  • Apply some tags
  • Refine this process until people like using it
  • Trial using this process at a Core sprint

User interface changes

Adding a new "Core Review Bonus" tag for core issues.

API Changes

None.

Comments

thedavidmeister’s picture

Issue tags: +Core Review Bonus
thedavidmeister’s picture

Issue summary: View changes

adding linke to "Core Review Bonus" search page

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Title: [meta] Core review bonus » [meta] [experimental] Core review bonus
thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

markhalliwell’s picture

Um... so we get points for what exactly? What do I get to do with them, brag that "I have more core bonus points than you"? How is that an "incentive"? I have never liked the PAReview Bonus "program". It serves as yet another "gate" to prevent contribution and in reality doesn't help us with the over all issue: commit rejection (in some for or another). A lot of these "rejections" are for trivial coding (or general) standards. These could be automated, if we really want to put forth the effort... but no one seems to. Sad. We already have enough GK for core, why are we adding yet another one?

thedavidmeister’s picture

@Mark Carver, please read the issue summary again, individuals don't get points, issues do, and issues with more points are reviewed first - see step 1 of "choosing an issue".

If you think that's how this works, I dare say you haven't actually participated in the PAReview Bonus? Neither system leads to an accrual of persistent "points" for a user. Any points that you "earn" by reviewing one issue have to be immediately put towards raising the visibility of another issue for potentially expedited review.

I'm 100% in support for automating coding standards reviews (you can see that I've left comments on the issue that you linked, supporting it back in July, no I haven't submitted a patch!), this issue isn't about changing the way we ask people to do reviews, it's an alternate way to help people doing reviews already select what to review next.

@Mark Carver, I don't think you fully understand the system proposed here (which is fine, ping me in chat to discuss if you want), nor its intention which is:

- Improve visibility of issues that need review, but are at risk of "slipping through the cracks"
- Encourage contributors to be more broad in what they review and consider reviewing other people's patches before they complain that nobody will review theirs
- Provide an alternate channel for contributors who want their patch reviewed to solicit a review outside IRC
- Provide some extra community spirit and feel-good vibes for people doing reviews
- Try to keep issues actionable by the main "stake holders" who have already put in valuable time and effort into getting the issue to CNR and just need some "outside" help to get it over the line

Regardless, the whole thing is entirely opt-in and can be safely ignored by anyone who doesn't want to participate, there's no plans at all to try and turn this into a Gate or any other kind of barrier to getting something over the line.

markhalliwell’s picture

Fair enough and you're right, I haven't participated in PAReview bonuses (just have heard poor experiences from them because it is used there as a gate). It still just seems like a lot of overhead in general though and there has to be a better solution than this.

thedavidmeister’s picture

#5 yeah, there's a bit of copying and pasting and some basic math involved, but compared to the overall amount of work that's involved in doing a proper review, it's pretty minor. The whole thing is super DIY, experiment-y atm - if it ever really took off i'm sure it could be "baked in" to the issue tracker (or Dreditor, more likely), but seriously, right now it's just me tagging issues and doing some reviews that I probably would have done anyway. I feel like it's waaaay too early to be talking about optimising the labour involved in a completely unproven process.

I had a great experience with the PAReview bonus, I know other people don't like it, I guess in many ways it comes down to personal expectations and experiences.

Bit of a downer that the first feedback I got was decidedly negative, but I want to keep going because I think it's too early for anyone to pass judgement objectively. I've done both, and Core reviews are a rather different beast to Project Application reviews, so it will be an interesting experiment I think :)

markhalliwell’s picture

Sorry :( Just re-read my comment and can see how it might come off as "negative". I really wasn't trying to be, just more confused than anything is all I guess. I am actually happy that at least someone is taking this challenge on though :) I'll definitely keep this in mind and will probably revisit this again soon to really read through and digest what it is you're trying to do and make suggestions for improvements. Keep up the great work @thedavidmeister!!

thedavidmeister’s picture

@Mark Carver, might I suggest that you actually work through the steps at least once, and have a look at how I've been doing it too for an example.

I think it will make more sense to you if you actually physically go through the process at least once, rather than trying to imagine it all.

markhalliwell’s picture

Time, especially to read all at a deeper comprehension level. I'll get to when I can.

thedavidmeister’s picture

no rush :)

thedavidmeister’s picture

I think I found something that could be improved.

Currently, you need to change the state of an issue for a review to "count", but really, looking at something like #1987410: [meta] system.module - Convert theme_ functions to Twig right now, one person doing everything required to change the state is a lot to ask:

- 30k patch, not huge, but not tiny
- Needs issue summary update
- Needs manual testing (no testing steps are in the summary)
- Needs profiling

So, probably, the requirement should be that you change the state of the issue OR do a review that removes a review related tag, if there are multiple review related tags on that issue (if you remove the last review related tag, you should also change the state of the issue).

I'll update the summary here, now.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary as per #11

thedavidmeister’s picture

I updated the issue summary, to hopefully make it easier to read, lowered the amount of work required to bump off a "Core Review Bonus" tag (#11) and also linked to the automated code sniff issue (#3).

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

jwilson3’s picture

Here's an idea, what if we *add* tags for issues that are *well done* in addition to adding tags for when things need assistance. (Eg, when you remove the "Needs issue summary" tag, you automatically add the "Has issue summary" tag). This will help people to start actively looking for issues that are in "needs review" and "has issue summary". (A small step, and only tangentially related to this but might prove helpful if core contributors were to get on board with it).'

I do like this idea -- having a couple core patches that I've worked on lingering in Needs review myself, and feeling the frustration mentioned in the issue summary.

thedavidmeister’s picture

@jwilson3, the change to tags sounds interesting. You should open up a meta for discussion, anything with potential to help get the number of CNR issues down would be great. The total number of CNR issues has actually *increased* since I opened this issue to 2427, we're going backwards at a rate of more than 1 unattended CNR issue per day :(

I managed to get a couple more issues tagged for review bonus today (https://drupal.org/project/issues/search/drupal?status%5B%5D=8&issue_tag...), so if you could help get the ball rolling over there with a review or two and maybe even get one of your own issues that you mentioned tagged, that would be amazing.

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Ok, so I've been shopping this idea around casually with people I know through work and the usual reaction is to run away from the wall of text that is the issue summary - this issue needs a healthy dose of KISS if it's ever going to get anywhere.

thedavidmeister’s picture

Issue summary: View changes
thedavidmeister’s picture

Issue summary: View changes
thedavidmeister’s picture

Issue summary: View changes
thedavidmeister’s picture

Issue summary: View changes
Related issues: +#1410826: [META] Review bonus
thedavidmeister’s picture

thedavidmeister’s picture

Issue summary: View changes
thedavidmeister’s picture

Issue summary: View changes
thedavidmeister’s picture

Issue summary: View changes
thedavidmeister’s picture

Issue summary: View changes
thedavidmeister’s picture

Issue summary: View changes
thedavidmeister’s picture

Issue summary: View changes
thedavidmeister’s picture

Issue summary: View changes
thedavidmeister’s picture

Issue summary: View changes
thedavidmeister’s picture

Ok, I think the issue summary is much easier to read now, and the whole process has been simplified.

Gone is the concept of "points" and whatever. What we have now couldn't really be simpler I don't think:

You leave a review for someone else (prioritising issues with the tag), you reference the issue you want reviewed, you tag the issue you want reviewed and reference the review that you did yourself.

What does everyone think of that?

xjm’s picture

Interesting proposal!

One thing in the summary seems misleading to me:

Automated reviews are useful (keep doing them until #1299710: [meta] Automate the coding-standards part of patch review lands!), but people appreciate it most when you take the effort to do a manual review alongside a scripted one.

I'd phrase this differently. We don't need people to be computers -- we have computers for that. We need people to be people. :) Each patch that gets committed to core needs to be read by human beings for code review, and tested by them manually because no automated test suite can cover everything.

So, if you're reviewing a patch and wanting others to return the favor, you should really be thoughtful about it. Read the issue, read the code, apply the patch and try it, do research to understand it better. Not everyone will do all these things for every issue, of course, but simply running an automated tool against someone's patch and not focusing on the actual issue tends to cause conflict and disappointment. More thoughts on this matter in my review guide blog post from a couple years ago. (One of these days I need to post a followup to all that... or distill it onto d.o so people don't reference my blog all the time.) ;)

Aside from that. I'd suggest removing the link to https://drupal.org/community-initiatives/drupal-core from the summary since it's not that relevant, and adding some to some of the relevant contributor task documents where appropriate:
https://drupal.org/contributor-tasks/review
https://drupal.org/contributor-tasks/manual-testing
https://drupal.org/contributor-tasks/improve-patch-standards
https://drupal.org/contributor-tasks/write-issue-summary

And, one of these days, we also need to make https://drupal.org/patch/review a whole lot better too. :)

Thanks so much for all your work on this proposal. This is still the #1 biggest hurdle for participating in core development and the biggest need core has.

thedavidmeister’s picture

Issue summary: View changes

Automated reviews are useful (keep doing them until #1299710: [meta] Automate the coding-standards part of patch review lands!), but people appreciate it most when you take the effort to do a manual review alongside a scripted one. A review that is purely automated and no human has read over the patch or tried to understand the problem that the patch solves, while useful, should not be used as part of the Core Review Bonus - See comment #30 in this thread.

Updated the issue summary to say this.

Removed the initiatives link. Added the suggested links.

@xjm, Thanks for the kind words :) would you consider trialling the system and tagging an issue next time you review a patch?

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

andypost’s picture

Version: 8.9.x-dev » 9.4.x-dev

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone’s picture

Title: [meta] [experimental] Core review bonus » [policy, no patch] Core review bonus
Category: Task » Plan

Changing to a policy to determine if the community agrees with this proposal.

Although there has been no activity here for 9 years it is worth checking if anyone is interested. I will ask in #contirbute.

quietone’s picture

Status: Active » Closed (outdated)

I asked about this issue in #contribute, both larowlan and catch replied. It was pointed out that the work done in the #needs-review-queue-initiative has made significant progress on reducing the number of issues waiting for review. As I write this there are 76 core issues at NR. Another point was that the proposal doesn't seem workable for core. Core has many topics where there reviewer needs to know about that topic to make a meaningful review. So, even you have done the work and have tagged an issue 'core review bonus' there may not be anyone participating in the program that can review it.

Given, those two points and the fact that there has been no activity here for 9 years, I am closing this as outdated.

To everyone who worked on this, thank you for the proposal and working to improve Drupal!

quietone’s picture