Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: add action to close stale PRs #48051

Closed
wants to merge 11 commits into from
Closed

Conversation

mhdawson
Copy link
Member

Add action to close PRs that are over 1 year old and have had no comments or updates in the last 6 months.

Fixes: #42981

Add action to close PRs that are over 1 year old and
have had no comments or updates in the last 6 months.

Fixes: nodejs#42981

Signed-off-by: Michael Dawson <mdawson@devrus.com>
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/actions
  • @nodejs/tsc

@nodejs-github-bot nodejs-github-bot added the meta Issues and PRs related to the general management of the project. label May 17, 2023
@mhdawson
Copy link
Member Author

mhdawson commented May 17, 2023

We currently have over 195 pull requests which were created more than a year ago and (up from 117 a year ago). Most of those have not had an update/comments for over 6 months.

This PR adds an action which will mark pull requests older than 1 year as stale once there have been no comments/updates for 5 months and then close them after another month if that does not trigger any comments.

I think this will help us lower the backlog of PRs (430 right) now) and in particular by helping close those which are very unlikely to land. A simple comment is all it takes to reset the PR so that it won't be closed if that is the right thing to do.

I had to add a new option to actions/stale to support the 1 year old check (end date - actions/stale#1013) so this uses my version of that action until we get the updated landed in actions/stale.

The current action also requires PRs to be tagged with the test-stale-pr lable so that it will only work on PRs on which that lable is added. I'll use that to slowly add to the list of PRs that will be processed by the new action. Once we are confident it's working ok I'll then add the tag to a few batches a week so that we don't get too many notifications all at once.

Once all PRs that are currently stale are processed I'll then remove the requirement for that lable in a follow on PR and we will be at steady state where PRs get processed as they reach the 1year old and 6 months without any updates point.

Co-authored-by: mscdex <mscdex@users.noreply.github.com>
mhdawson and others added 2 commits May 17, 2023 17:41
Co-authored-by: mscdex <mscdex@users.noreply.github.com>
Co-authored-by: mscdex <mscdex@users.noreply.github.com>
@Trott
Copy link
Member

Trott commented May 18, 2023

Can this be a modification of .github/workflows/close-stalled.yml rather than creating a new job? We already have that and .github/workflows/close-stale-feature-requests.yml and I would rather have these things consolidated into a single action rather than spread out like this. (And if we need to have multiple actions, let's name them clearly. I would think close-stalled would handle stalled PRs, but if not, then let's rename it.)

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

.github/workflows/close-stale-pull-requests.yml Outdated Show resolved Hide resolved
.github/workflows/close-stale-pull-requests.yml Outdated Show resolved Hide resolved
.github/workflows/close-stale-pull-requests.yml Outdated Show resolved Hide resolved
@mhdawson
Copy link
Member Author

@Trott testing actions is a bit difficult and I'd be a bit worried that by combining in I might break the existing action or cause issues we don't want to be processed covered. If we want to consolidate then maybe we can do that as a follow on step as we already have more than one action handling stale issues.

mhdawson and others added 4 commits May 18, 2023 09:05
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
Signed-off-by: Michael Dawson <mdawson@devrus.com>
@mhdawson
Copy link
Member Author

@nodejs/tsc FYI that I plan to land this in the next day or so unless there are comments/objections.

mhdawson added a commit that referenced this pull request May 26, 2023
Add action to close PRs that are over 1 year old and
have had no comments or updates in the last 6 months.

Fixes: #42981

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #48051
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@mhdawson
Copy link
Member Author

Landed in 65be890

@mhdawson mhdawson closed this May 26, 2023
mhdawson added a commit to mhdawson/io.js that referenced this pull request May 26, 2023
My original plan of adding a lable to limit those initially
process as outlined in nodejs#48051
does not work I think because adding a lable updates the last
update time.

- Removing the need for the lable
- Remove the cron scheduling so that it only runs when I run it manually
- Fix the display name for the action as I missed updating that after
  cut and paste from existing action

The plan will be to find stop dates that should only affect a reasonable
number of PRs at a time and then run in batches using that instead.

Signed-off-by: Michael Dawson <mdawson@devrus.com>
targos pushed a commit that referenced this pull request May 30, 2023
Add action to close PRs that are over 1 year old and
have had no comments or updates in the last 6 months.

Fixes: #42981

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #48051
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@targos targos mentioned this pull request Jun 4, 2023
mhdawson added a commit that referenced this pull request Jun 5, 2023
My original plan of adding a lable to limit those initially
process as outlined in #48051
does not work I think because adding a lable updates the last
update time.

- Removing the need for the lable
- Remove the cron scheduling so that it only runs when I run it manually
- Fix the display name for the action as I missed updating that after
  cut and paste from existing action

The plan will be to find stop dates that should only affect a reasonable
number of PRs at a time and then run in batches using that instead.

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #48196
Reviewed-By: Mestery <mestery@protonmail.com>
RafaelGSS pushed a commit that referenced this pull request Jul 3, 2023
My original plan of adding a lable to limit those initially
process as outlined in #48051
does not work I think because adding a lable updates the last
update time.

- Removing the need for the lable
- Remove the cron scheduling so that it only runs when I run it manually
- Fix the display name for the action as I missed updating that after
  cut and paste from existing action

The plan will be to find stop dates that should only affect a reasonable
number of PRs at a time and then run in batches using that instead.

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #48196
Reviewed-By: Mestery <mestery@protonmail.com>
danielleadams pushed a commit that referenced this pull request Jul 6, 2023
Add action to close PRs that are over 1 year old and
have had no comments or updates in the last 6 months.

Fixes: #42981

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #48051
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
MoLow pushed a commit to MoLow/node that referenced this pull request Jul 6, 2023
Add action to close PRs that are over 1 year old and
have had no comments or updates in the last 6 months.

Fixes: nodejs#42981

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: nodejs#48051
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
Add action to close PRs that are over 1 year old and
have had no comments or updates in the last 6 months.

Fixes: nodejs#42981

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: nodejs#48051
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
My original plan of adding a lable to limit those initially
process as outlined in nodejs#48051
does not work I think because adding a lable updates the last
update time.

- Removing the need for the lable
- Remove the cron scheduling so that it only runs when I run it manually
- Fix the display name for the action as I missed updating that after
  cut and paste from existing action

The plan will be to find stop dates that should only affect a reasonable
number of PRs at a time and then run in batches using that instead.

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: nodejs#48196
Reviewed-By: Mestery <mestery@protonmail.com>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
Add action to close PRs that are over 1 year old and
have had no comments or updates in the last 6 months.

Fixes: nodejs#42981

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: nodejs#48051
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
My original plan of adding a lable to limit those initially
process as outlined in nodejs#48051
does not work I think because adding a lable updates the last
update time.

- Removing the need for the lable
- Remove the cron scheduling so that it only runs when I run it manually
- Fix the display name for the action as I missed updating that after
  cut and paste from existing action

The plan will be to find stop dates that should only affect a reasonable
number of PRs at a time and then run in batches using that instead.

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: nodejs#48196
Reviewed-By: Mestery <mestery@protonmail.com>
ruyadorno pushed a commit that referenced this pull request Aug 29, 2023
My original plan of adding a lable to limit those initially
process as outlined in #48051
does not work I think because adding a lable updates the last
update time.

- Removing the need for the lable
- Remove the cron scheduling so that it only runs when I run it manually
- Fix the display name for the action as I missed updating that after
  cut and paste from existing action

The plan will be to find stop dates that should only affect a reasonable
number of PRs at a time and then run in batches using that instead.

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #48196
Reviewed-By: Mestery <mestery@protonmail.com>
@avivkeller avivkeller mentioned this pull request Aug 31, 2024
4 tasks
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.

Auto closing old stale PRs
6 participants