Skip to content

Conversation

@aduh95
Copy link
Contributor

@aduh95 aduh95 commented Nov 2, 2025

The number of PRs keeps creeping up, one of the reason is that we have PRs that have been approved and just forgotten about. By having the author ready added automatically, it can help with that as I for one will often check out https://github.com/nodejs/node/pulls?q=is:pr+is:open+label:%22author+ready%22+-label:commit-queue e.g. when preparing a release.

Not that this won't add the label to all PRs that deserve it, won't remove it automatically, and might add to some that do not deserve it, so there will still be triagging work to do.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/actions

@nodejs-github-bot nodejs-github-bot added the meta Issues and PRs related to the general management of the project. label Nov 2, 2025
Copy link
Member

@legendecas legendecas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this recognize "open comment threads"? I personally usually use "comment" instead of "request for changes", so that the review won't be incorrectly interpreted as aggressiveness.

Though the collaborator guide does not require the author to address all the open comments, an automated process to add author ready label may incorrectly give contributor an impression that they can ignore the open comments and the PR is ready to land.

@legendecas
Copy link
Member

legendecas commented Nov 2, 2025

Oh, actually the guide does mention the PR should address outstanding review comments before marking it as author ready:

https://github.com/nodejs/node/blob/601208b671e0b4816ff5ebc601290569a6cdddd7/doc/contributing/collaborator-guide.md?plain=1#author-ready-pull-requests#L75

@aduh95
Copy link
Contributor Author

aduh95 commented Nov 3, 2025

It might make sense to change the definition of author ready to PR that might be ready: IMO a PR that has author ready PRs that have at least one approval, no pending requests for changes, and a CI started. should not be blindly added to the CQ.

Anyway, I don't mind adding more heuristics to decrease the chance of mislabelling, if you have more suggestions bring them on!

@aduh95
Copy link
Contributor Author

aduh95 commented Nov 8, 2025

FWIW, https://github.com/nodejs/node/pulls?q=is:pr+is:open+-label:%22author+ready%22+-label:commit-queue+-label:needs-ci+review:approved+ shows there are about 40 PRs that are likely mergeable; https://github.com/nodejs/node/pulls?q=is:pr+is:open+-label:%22author+ready%22+-label:commit-queue+label:needs-ci+review:approved+ lists 85 that would likely need a CI run before being merged. It's probably too much work for any individual, though if the triaging was done on a more frequent basis, it would be much easier, and my hope is that author ready PRs that have at least one approval, no pending requests for changes, and a CI started. would be part of the solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

meta Issues and PRs related to the general management of the project.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants