[Policy] Automatically check whether 'needs review' patches apply and pass linting

I am willing to authorize the use of a new official bot for this purpose.

The parameters would be that we follow the best practices of the project update bot:

• It has a way for the maintainer of any given project to opt out, for example. (In this case, we'd want to let someone opt out an individual issue perhaps).
• It also needs to be easily tunable for the amount of re-testing it's doing. The price of testing has gone up lately. It can vary between 75 cents and 1.20 an hour now.
• It should have a dedicated account, with a user description that explains what it is doing and where to file issues.
• Messages it posts should be clearly documented in issues, so that people can provide feedback.
• Lastly, it'd be a great help if someone could help capture any other known standards we follow for project update, and take those and the ones above, and help me create a new page under the Drupalorg documentation guide for 'bot standards'. It's something I've been meaning today for more than a year (and eventually will, but help is welcome).

That said - it wouldn't hurt for someone to check out the DrupalCI_testbot/PIFT codebase - just in case this turns out to be *really easy* to do using the existing RTBC logic. We'd certainly accept an MR.

It also needs to be easily tunable for the amount of re-testing it's doing. The price of testing has gone up lately. It can vary between 75 cents and 1.20 an hour now.

If we use nod_'s script this would be a non-issue - it does the patch linting checks on the machine its run on.

It has a way for the maintainer of any given project to opt out, for example. (In this case, we'd want to let someone opt out an individual issue perhaps).

This would only run against core. If we're only doing it as a one-off, then it seems a bit tricky to offer an opt-out, maybe we could respect a special issue tag though?

@Catch - sure - if we're limiting the scope in those ways, than that can serve in place of having those configuration options. Makes it easier certainly.

(Useful stuff described in our existing bot account: https://www.drupal.org/u/project-update-bot → )

Planning on running my script on tuesday 31st. What does the script does:

1. Get all Needs Review issues for major versions: 11, 10, 9
2. For each issue:
   • Find the last patch, either the most recent patch file upload or the most recent merge request. If there are patches and merge request on the same issue, use the most recent.
   • Download the patch or the diff from the merge request
   • Apply the patch locally on the corresponding major version (10.1.x for issues with 10* version, 9.5.x for issues with 9* version)
   • If the patch applies, run the commit checks
3. If the patch didn't apply or commit check failed, put the issue back in Needs Work with a message explaining what's going on. I'll try to look at submitting a file with the result of the patch apply or the commit check log.

FYI I get a 5xx error when trying to look at https://www.drupal.org/u/project-update-bot →

The page loads for me after a while, I think it's the 3,500 issue credits taking a while.

Copy and pasting from there:

This account was created by the Drupal Association to help upgrade contributed projects to Drupal 9 and then to Drupal 10. Currently it provides patches generated by Drupal Rector to remove Drupal 10 deprecated API uses. All of those issues can be found at https://www.drupal.org/project/issues/search?issue_tags=ProjectUpdateBotD10 →

If you are a project maintainers there are a few options for dealing with issues created by this bot:

Leave the issue open and apply the provided patch to remove some or all Drupal 10 deprecated API uses. The Project Update Bot will check weekly if Drupal Rector is able to remove new deprecations and post a new patch if possible.
Remove the “ProjectUpdateBotD10” tag from the issue to stop new patches from being posted. If you would like to use the issue and the patch as a starting point simply remove this tag and the bot will not post any new patches. Add the tag back and the bot will post new patches if possible.
Close the issue to stop the bot from posting new patches. If you are already handling deprecated API uses in another issue or otherwise don’t find the patches helpful simply close the issue and the bot will not post any new patches.

If want to respect a tag, something like NoNeedsReviewBot might work - although really that's only going to be relevant if we do a second run. I can try to draft some copy for this tomorrow.

Rough drafts for verbiage added to the issue summary - one for the user account, one for the issue comment.

Excellent, thanks

Nice rough drafts. I suggest adding to the comment a link to the Contributor Task. "Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues".

Some of the sentences are long and complicated so I made an attempt to convert the content to plain English. There is room for improvement but maybe it is of some use. Here are the results:

The Needs Review Queue Bot is a project of the Needs Review Queue Initiative. Join the Drupal Slack channel, #needs-review-queue-initiative, to find out more about the initiative. The initiative goal is to reduce the number of issues needing a review, which was 2600 when the initiative started.

The Needs Review Queue Bot performs limited testing on issues marked "Needs review". The Drupal CI does not automatically test these issue. One reason is that the cost of doing a full test run on these issues every 48 hours would be prohibitive.

The Needs Review Queue Bot only tests if a patch or MR applies to the issue target branch and runs Drupal's commit checks. If either of these steps fails, the bot will add a comment and mark the issue "Needs work".

Some issues need to stay at "Needs review" despite the existing patch or MR being out of date. In these cases, add the tag "no-needs-review-bot". The Needs Review Queue Bot ignores issues with the tag, "no-needs-review-bot".

This is a temporary measure. We hope that DrupalCI or GitLab CI adds similar automated re-linting for needs review issues.

The Needs Review Queue Bot → tested this issue. It either no longer applies to the branch of Drupal core this issue is against, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

I'm all set up now, used the text from #49 with the edit from #50 for the issue comment.

If we're ok with the text, now it's just a matter of deciding when to run things on the 1045 issues concerned. It should take about 6 hours.

The bot will run in about 4 hours

The script started, it should take about 5 hours to run provided the bot doesn't get blocked by d.o security set up :)

To see all the issues impacted by the bot you can check look at this issue list: https://www.drupal.org/project/issues/search/drupal?project_issue_follow... →

Total issues: 2167
Issues with no patches: 43
Number of patches not applying: 802
Number of commit check failed: 176

Hi team, love the effort going into reducing the queue!

A little feedback on the bot that might a bit too little too late: when the patch doesn't apply, the txt file doesn't really make that clear by just including the patch link in a json format. I was pretty confused by it and predict the same for plenty of contributors.

Just leaving this here in case the bot becomes a regular fixture :)

Seconding @darvanen above. Also it would be nice to change the comment based on the failure too.

I lost a bit of time on the issues I follow today trying to figure out which failure each issue was being pushed back on (i.e failed to apply or linting).

If the contents of the file change based on that then hopefully it wouldn't be much work to change the comment too.

Thanks for the great work as always :)

Script finished properly. It did break email notifications for a while. Review Queue is at 1182 and 976 issues were put back in NW by the bot.

If we run the bot again I'll need to make it honor the needs-review tag and fix up the text of the issue. You can see what type of failure is it by the size of the text file. if it's aournd 100-ish bytes it's that the patch doesn't apply, bigger than that you have commit check failures.

With gitlab make major jumps recently, wonder if this can be closed?

I bet no, it still needs some bot to schedule jobs in the latest pipeline or to create new pipeline

https://docs.gitlab.com/ee/ci/pipelines/schedules.html

I opened a couple of gitlab-specific issues about this one: 📌 [PP-1] MR retesting with GitLab CI Postponed and 📌 Mark issues needs work when MRs can't be merged or pipelines fail Active , mostly about RTBC issues.

I think the equivalent of this issue on gitlab might be something like https://gitlab.com/gitlab-org/gitlab-foss/-/issues/18834, which would in turn trigger a new test run on the MR.

With merge requests, the equivalent to “no longer applies” would be “needs manual rebase.” GitLab and Drupal.org now show the MR’s ability to merge status prominently, for example, “Related merge requests” at https://gitlab.com/gitlab-org/gitlab/-/issues/423114. However, I don’t see anything in GitLab’s issue listing, https://gitlab.com/gitlab-org/gitlab/-/issues/?sort=updated_desc&state=o.... The merge request listing does surface the pipeline results, but I don’t see a filter for it, https://git.drupalcode.org/project/drupal/-/merge_requests

At some point, maintainers may want to consider if they want to look at the issues or merge requests listing primarily for code review. I suspect it may stay as issues, since policies and plans don’t land as code merges.

https://gitlab.com/gitlab-org/ruby/gems/gitlab-triage is what GitLab (the company) runs on top of GitLab (the software) for automated labeling. I don't see pipeline status documented as a condition.

With patches now gone should this be closed?

I do not think that currently the failing MRs or MRs which needs to be rebased, are moved to NW automatically (except by NR queue bot). Probably this is more relevant issue: 📌 Mark issues needs work when MRs can't be merged or pipelines fail Active , but since this is a policy issue, I am not sure we should close this without a decision.