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: close stalled issues and PRs with GitHub Action #34555

Merged
merged 0 commits into from
Sep 3, 2020

Conversation

phillipj
Copy link
Member

This introduces a GitHub Action workflow to close issues and PRs which has been labelled stalled 30 days ago (or more).

stale labelling and unlabelling of issues and PRs are still done manually by collaborators.

For the curious ones, all the stale action options are described here: stale/action.yml.

Refs nodejs/github-bot#261

Checklist

@nodejs-github-bot nodejs-github-bot added the meta Issues and PRs related to the general management of the project. label Jul 29, 2020
@phillipj
Copy link
Member Author

Hmz, which subsystem this belongs to was obvious to me.. Looking at the bot's label choice, I should have used meta?

@phillipj
Copy link
Member Author

Example of this in practise can be seen in phillipj/stale-action-with-manual-labelling. Only difference being days since being labelled stalled is set to 1 instead of 30.

@mscdex
Copy link
Contributor

mscdex commented Jul 29, 2020

Should this be leaving a message when closing?

@mmarchini
Copy link
Contributor

Hmz, which subsystem this belongs to was obvious to me.. Looking at the bot's label choice, I should have used meta?

build is fine, the labels applied by the bot doesn't necessarily reflect the actual subsystem (we might want to change the bot to label .github/workflows with build though).

@phillipj
Copy link
Member Author

Should this be leaving a message when closing?

I like the idea and is luckily configurable with close-issue-message | close-pr-message. Some kind of explanation is probably better than nothing and opening up for questions if any is good etiquette, right?

Closing this because it has stalled. Feel free to ping the collaborator who labelled it stalled if you have any questions.

Does that sound good to you @mscdex? Or du you have a more eloquent alternative?

@mmarchini
Copy link
Contributor

Suggestion for the comments: Closing this because it has stalled. Feel free to reopen if this issue/PR is still relevant, or to ping the collaborator who labelled it stalled if you have any questions.

@lundibundi
Copy link
Member

IMO it would be also cool to have an automatic comment that it was marked stalled and will be closed in 30 days as adding a label AFAIK doesn't trigger a GitHub notification and people will only know about it when the issue/PR is already closed.

Copy link
Member

@lundibundi lundibundi left a comment

Choose a reason for hiding this comment

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

LGTM after adding close-issue-message/close-pr-message

@lundibundi
Copy link
Member

Also ping @Trott for closing message suggestions 😄.

@mmarchini
Copy link
Contributor

mmarchini commented Jul 30, 2020

@lundibundi it would be a good idea, but it wouldn't work for pull requests (because the vast majority comes from forks, which means the action that runs when a label is added to the PR won't have write access or access to secrets) and therefore it would be inconsistent between issues and PRs. We'll need to trust collaborators to leave a comment when labeling something as stalled

@mscdex
Copy link
Contributor

mscdex commented Jul 30, 2020

@phillipj I don't have any particular preference on the wording but I think there should be some kind of explanation provided so that everyone understands why the close happened. Doing so would keep things in line with how manual closes have occurred on past issues/PRs.

@lundibundi
Copy link
Member

@mmarchini That's unfortunate, though I think it would be better to at least have the message in the issues (and IMO it's more important for issues than it is for PRs) than to not have it in neither.
Not a blocker anyways just a suggestion/possible follow-ups.

@phillipj
Copy link
Member Author

phillipj commented Jul 30, 2020

@lundibundi great suggestion for a follow up if we see this auto closing behaviour confuses contributors.

Although a bit tricky, it might be feasible.. Sounds a bit like what I've noticed the remove-stale-when-updated option activates; it goes through all stalled issues/PRs and removes the label if any comments are posted.

As far as I've understood, it circumvents what @mmarchini is raising by running on a cron schedule rather than reacting to action events like push | labeled etc.

Feels like something similar could be done to achieve what you're suggesting.

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

@phillipj
Copy link
Member Author

phillipj commented Aug 9, 2020

Let me know if there's anything expected of me before this can land. I don't have any upcoming changes in mind..

@mmarchini
Copy link
Contributor

Was about to land it and create the stalled label. Turns out we already have that label and we have 44 issues and PRs with it: stalled Issues and PRs that are stalled. . Do we want the 30 days to start counting once we land this PR, or when the label was originally added? If the former, we need to remove and re-add the label before landing this.

@mmarchini
Copy link
Contributor

There's a new GitHub Actions event which can be used on Pull Requests and has proper write access: pull_request_target. @phillipj can you add another Action which will automatically leave a comment when the label is added? Something like:

This issue/PR was marked as stalled by , it will be automatically closed in 30 days. If it should remain open, please leave a comment explaining why it should remain open.

To run the action only when specific labels are added, you can do:

on:
  issue:
    types: [labeled]
  pull_request_target:
    types: [labeled]

jobs:
  staleComment:
    runs-on: ubuntu-latest
    if: github.event.label.name == 'stalled'

Here's an example.

.github/workflows/comment-stalled.yml Outdated Show resolved Hide resolved
Comment on lines 7 to 6
pull_request_target:
types: [labeled]
Copy link
Contributor

Choose a reason for hiding this comment

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

This won't work for existing PRs, but maybe that's ok.

Copy link
Member Author

Choose a reason for hiding this comment

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

I fumbled around quite a bit myself before discovering that as well... You know ways to circumvent that by any chance?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not yet :/

@phillipj
Copy link
Member Author

As far as I'm concerned, this is the only unanswered question from @mmarchini:

Do we want the 30 days to start counting once we land this PR, or when the label was originally added? If the former, we need to remove and re-add the label before landing this.

Who can answer this? Is it a TSC concern?

@mmarchini
Copy link
Contributor

@nodejs/tsc question: this PR will auto-close issues and PRs 30 days after the staled label is added to them (the label will be added only manually, by collaborators and triage team). We have quite a few issues and PRs with the label already, do we want to close those 30 days from when the label was added (which might mean closing them right after this PR lands), or do we want those to be closed 30 days after this PR lands?

@targos
Copy link
Member

targos commented Aug 18, 2020

If it's the second option, will there be an automatic message posted to the issues which have the label?

@mmarchini
Copy link
Contributor

mmarchini commented Aug 18, 2020

If it's the second option, will there be an automatic message posted to the issues which have the label?

Yes for Issues, no for PRs (because we're using the pull_request_target event, which needs to be present in the PR git log for it to run, so it will only run on rebased PRs or new PRs. We can't use pull_request event because it doesn't have permission to comment).

@mscdex
Copy link
Contributor

mscdex commented Aug 18, 2020

We can't use pull_request event because it doesn't have permission to comment).

Isn't that a simple fix that could be made by a repo admin?

@mmarchini
Copy link
Contributor

mmarchini commented Aug 18, 2020

No, that's a built-in limitation with the pull_request event

@Trott
Copy link
Member

Trott commented Aug 21, 2020

I mildly favor removing and re-adding the label so that the 30 days starts after the PR lands. But I also believe that people doing the work get to make the decisions when appropriate, and I think it's appropriate here, so if you want to auto-close those right away, that's fine too. People will get notifications and can always re-open them and/or remove the label and/or comment.

@github-actions github-actions bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Sep 2, 2020
@github-actions
Copy link
Contributor

github-actions bot commented Sep 2, 2020

Commit Queue failed
- Loading data for nodejs/node/pull/34555
✔  Done loading data for nodejs/node/pull/34555
----------------------------------- PR info ------------------------------------
Title      build: close stalled issues and PRs with GitHub Action (#34555)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     phillipj:close-stalled-github-action -> nodejs:master
Labels     meta
Commits    4
 - build: close stalled issues and PRs with github action
 - fixup! build: close stalled issues and PRs with github action
 - build: comment about auto close when stalled via with github action
 - fixup! build: comment about auto close when stalled via with github a…
Committers 1
 - Phillip Johnsen 
PR-URL: https://github.com/nodejs/node/pull/34555
Reviewed-By: James M Snell 
Reviewed-By: Mary Marchini 
Reviewed-By: Denys Otrishko 
Reviewed-By: Gus Caplan 
Reviewed-By: Zeyu Yang 
Reviewed-By: Michael Dawson 
Reviewed-By: Shelley Vohr 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/34555
Reviewed-By: James M Snell 
Reviewed-By: Mary Marchini 
Reviewed-By: Denys Otrishko 
Reviewed-By: Gus Caplan 
Reviewed-By: Zeyu Yang 
Reviewed-By: Michael Dawson 
Reviewed-By: Shelley Vohr 
--------------------------------------------------------------------------------
   ✔  Last GitHub Actions successful
   ℹ  Last Full PR CI on 2020-09-02T22:35:12Z: https://ci.nodejs.org/job/node-test-pull-request/33025/
- Querying data for job/node-test-pull-request/33025/
✔  Build data downloaded
- Querying failures of job/node-test-commit/40579/
✔  Data downloaded
   ✖  2 failure(s) on the last Jenkins CI run
   ℹ  This PR was created on Wed, 29 Jul 2020 18:52:59 GMT
   ✔  Approvals: 7
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/34555#pullrequestreview-457802366
   ✔  - Mary Marchini (@mmarchini) (TSC): https://github.com/nodejs/node/pull/34555#pullrequestreview-457930033
   ✔  - Denys Otrishko (@lundibundi): https://github.com/nodejs/node/pull/34555#pullrequestreview-458181195
   ✔  - Gus Caplan (@devsnek): https://github.com/nodejs/node/pull/34555#pullrequestreview-458544936
   ✔  - Zeyu Yang (@himself65): https://github.com/nodejs/node/pull/34555#pullrequestreview-458629607
   ✔  - Michael Dawson (@mhdawson) (TSC): https://github.com/nodejs/node/pull/34555#pullrequestreview-458746991
   ✔  - Shelley Vohr (@codebytere) (TSC): https://github.com/nodejs/node/pull/34555#pullrequestreview-481308862
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

@mmarchini
Copy link
Contributor

i'll just go ahead and will land it manually 🙃

mmarchini pushed a commit that referenced this pull request Sep 3, 2020
This introduces a GitHub Action workflow to close issues and PRs
which has been labelled `stalled` 30 days ago (or more).

`stale` labelling and unlabelling of issues and PRs are still done
manually by collaborators.

Refs nodejs/github-bot#261

PR-URL: #34555
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Mary Marchini <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
@mmarchini mmarchini closed this Sep 3, 2020
@mmarchini mmarchini merged commit b23a932 into nodejs:master Sep 3, 2020
@mmarchini
Copy link
Contributor

Landed in cc9ac42...b23a932

@phillipj phillipj deleted the close-stalled-github-action branch September 3, 2020 06:57
richardlau pushed a commit that referenced this pull request Sep 3, 2020
This introduces a GitHub Action workflow to close issues and PRs
which has been labelled `stalled` 30 days ago (or more).

`stale` labelling and unlabelling of issues and PRs are still done
manually by collaborators.

Refs nodejs/github-bot#261

PR-URL: #34555
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Mary Marchini <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
richardlau pushed a commit that referenced this pull request Sep 3, 2020
As part of automatically closing issues and PRs 30 days after they got
labelled with `stalled`, these changes adds a GitHub Action workflow
posting a comment information about what will happen in 30 days upon
being labelled.

PR-URL: #34555
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Mary Marchini <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
@richardlau richardlau mentioned this pull request Sep 3, 2020
4 tasks
addaleax pushed a commit that referenced this pull request Sep 22, 2020
This introduces a GitHub Action workflow to close issues and PRs
which has been labelled `stalled` 30 days ago (or more).

`stale` labelling and unlabelling of issues and PRs are still done
manually by collaborators.

Refs nodejs/github-bot#261

PR-URL: #34555
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Mary Marchini <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
addaleax pushed a commit that referenced this pull request Sep 22, 2020
As part of automatically closing issues and PRs 30 days after they got
labelled with `stalled`, these changes adds a GitHub Action workflow
posting a comment information about what will happen in 30 days upon
being labelled.

PR-URL: #34555
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Mary Marchini <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
addaleax pushed a commit that referenced this pull request Sep 22, 2020
This introduces a GitHub Action workflow to close issues and PRs
which has been labelled `stalled` 30 days ago (or more).

`stale` labelling and unlabelling of issues and PRs are still done
manually by collaborators.

Refs nodejs/github-bot#261

PR-URL: #34555
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Mary Marchini <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
addaleax pushed a commit that referenced this pull request Sep 22, 2020
As part of automatically closing issues and PRs 30 days after they got
labelled with `stalled`, these changes adds a GitHub Action workflow
posting a comment information about what will happen in 30 days upon
being labelled.

PR-URL: #34555
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Mary Marchini <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
@codebytere codebytere mentioned this pull request Sep 28, 2020
joesepi pushed a commit to joesepi/node that referenced this pull request Jan 8, 2021
This introduces a GitHub Action workflow to close issues and PRs
which has been labelled `stalled` 30 days ago (or more).

`stale` labelling and unlabelling of issues and PRs are still done
manually by collaborators.

Refs nodejs/github-bot#261

PR-URL: nodejs#34555
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Mary Marchini <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
joesepi pushed a commit to joesepi/node that referenced this pull request Jan 8, 2021
As part of automatically closing issues and PRs 30 days after they got
labelled with `stalled`, these changes adds a GitHub Action workflow
posting a comment information about what will happen in 30 days upon
being labelled.

PR-URL: nodejs#34555
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Mary Marchini <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
@targos targos removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Sep 5, 2021
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.