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

[skip-ci] Lock closed issues and PRs #19691

Merged
merged 2 commits into from
Aug 22, 2023

Conversation

cevich
Copy link
Member

@cevich cevich commented Aug 21, 2023

Lock closed issues and PRs after a reasonable amount of time to prevent spamming engineers with irreverent notifications. Should the workflow fail for some reason, notify the podman-monitor mailing list.

N/B: This PR includes a commit intended to help confirm this workflow functions w/o affecting potentially thousands of issues/PRs. It's intended to be reverted by a future PR.

Does this PR introduce a user-facing change?

None

@openshift-ci openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none labels Aug 21, 2023
@cevich
Copy link
Member Author

cevich commented Aug 21, 2023

Testing this workflow in https://github.com/containers/automation_sandbox:

Investigating fix...

@cevich cevich force-pushed the lock_closed_issues branch from 7c90268 to b36c62c Compare August 21, 2023 19:13
@cevich
Copy link
Member Author

cevich commented Aug 21, 2023

...fixed by adding permissions. Manually tested the fix and now the workflow now behaves as intended.

Edit: Clarify link

@cevich cevich changed the title [WIP] [skip-ci] Lock closed issues [skip-ci] Lock closed issues and PRs Aug 21, 2023
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 21, 2023
@cevich
Copy link
Member Author

cevich commented Aug 21, 2023

@containers/podman-maintainers PTAL, this should be ready to go. It will need to be manually merged. I tested this as best as I could (see above), but there could be hiccups. I intend to open followup PRs as needed once we see what happens.

@edsantiago
Copy link
Member

I'm confused. In your test run (I think that's your test run?) I see 1825, but then I see that recent PRs are also tagged with the lock. I was expecting only old-old PRs to be locked. Have I misunderstood something?

@cevich
Copy link
Member Author

cevich commented Aug 21, 2023

I was expecting only old-old PRs to be locked. Have I misunderstood something?

Thanks for checking closely. I didn't look at the activity dates, since all/most of the PRs in that repo. are really really old. I'll go look and make sure they were really over 90-days old...

@edsantiago
Copy link
Member

The ones I see are >90 days, but I was expecting 1825 because the magic clicky-expandy log that I can't link to says 1825.

@cevich
Copy link
Member Author

cevich commented Aug 21, 2023

...yeah, the most recent PR did not get the label. I think it's okay, there were no issues/PRs older than 1825 to close.

@cevich
Copy link
Member Author

cevich commented Aug 21, 2023

In case it's not clear to others, this is the test run where I changed it to 90-days. It just happened there was only one closed PR newer than 90 days.

Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

It would be nice to drop a message explaining when an issue/PR gets locked.

Something down the lines of "Closed issues/PRs are getting locked after 90 days. Please open a new issue with the requested issue template if needed" .

Email filters can help in marking the bot message as read, so we don't get spammed by it. The reasoning for such a message is that I think most users are unaware that we prefer/need fresh issues rather than discussing on old ones. Some guidance may be required.

@Luap99
Copy link
Member

Luap99 commented Aug 22, 2023

It would be nice to drop a message explaining when an issue/PR gets locked.

Something down the lines of "Closed issues/PRs are getting locked after 90 days. Please open a new issue with the requested issue template if needed" .

Email filters can help in marking the bot message as read, so we don't get spammed by it. The reasoning for such a message is that I think most users are unaware that we prefer/need fresh issues rather than discussing on old ones. Some guidance may be required.

That is why we apply the label locked - please file new issue/PR, I really do not want to generate new messages. Otherwise this sill spams the hell out of me. I am using the build in github notifications, not email, so I cannot use such a filter.
I discussed this but you didn't answer on #19012 (reply in thread) so I assume @cevich just went ahead with this approach.

The locking and applying label part does not create a new notification. Maintainers may create filters for such messages but it will still generate noise for the other participants on the issue.


@cevich I think you should also update CONTRIBUTING.md to mention this behaviour.

@vrothberg
Copy link
Member

I discussed this but you didn't answer on #19012 (reply in thread) so I assume @cevich just went ahead with this approach.

Sorry, that went through the cracks.

I am using the build in github notifications, not email, so I cannot use such a filter.

I see that. It won't work using the GitHub web UI. OK, then please ignore my comment; I missed my chance.

@cevich cevich force-pushed the lock_closed_issues branch from b36c62c to f3e3c0b Compare August 22, 2023 14:58
@cevich
Copy link
Member Author

cevich commented Aug 22, 2023

Force-push: Updated CONTRIBUTING.md as requested.

CONTRIBUTING.md Outdated Show resolved Hide resolved
cevich added 2 commits August 22, 2023 13:30
This commit limits the blast-radius should the workflow fail
catastrophically.  It also instruments the workflow with a job-level
test-failure to trigger a notification mail.  This commit should be
reverted once the workflow is deemed functional.

Signed-off-by: Chris Evich <[email protected]>
@cevich cevich force-pushed the lock_closed_issues branch from f3e3c0b to f0e8e79 Compare August 22, 2023 17:30
@cevich
Copy link
Member Author

cevich commented Aug 22, 2023

Force-push: Updated CONTRIBUTING.md again.

@mheon
Copy link
Member

mheon commented Aug 22, 2023

LGTM, very welcome change

@rhatdan
Copy link
Member

rhatdan commented Aug 22, 2023

/lgtm
/approve

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 22, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 22, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cevich, rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 22, 2023
@openshift-merge-robot openshift-merge-robot merged commit 5e295a9 into containers:main Aug 22, 2023
@cevich
Copy link
Member Author

cevich commented Aug 22, 2023

@cevich
Copy link
Member Author

cevich commented Aug 22, 2023

Result: The latest opened issue (#1313), closed on Aug 22, 2018 was correctly locked. Only 49 other issues were touched (limit built-in to the action itself). Then the workflow test-failed as designed, and the podman-monitor list received the test-failure notice.

@edsantiago
Copy link
Member

Looks good. The issue list shows suitable old IDs, and all of them are closed or merged:

$ jq '.[].issue_number' < foo.js | while read i;do printf "%6d %s\n" $i "$(gh issue view $i|grep state:)";done
  1313 state:   CLOSED
  1314 state:   CLOSED
  ....

cevich added a commit to cevich/buildah that referenced this pull request Aug 22, 2023
cevich added a commit to cevich/buildah that referenced this pull request Aug 22, 2023
cevich added a commit to cevich/buildah that referenced this pull request Aug 22, 2023
Lock discussions on closed PRs and Issues after 90-days of inactivity.

Ref:
     containers/podman#19012
and
     containers/podman#19691
and
     containers/podman#19700

Signed-off-by: Chris Evich <[email protected]>
cevich added a commit to cevich/skopeo that referenced this pull request Aug 22, 2023
Lock discussions on closed PRs and Issues after 90-days of inactivity.

Ref:
     containers/podman#19012
and
     containers/podman#19691
and
     containers/podman#19700

Signed-off-by: Chris Evich <[email protected]>
@Luap99
Copy link
Member

Luap99 commented Aug 23, 2023

Thanks!

@edsantiago
Copy link
Member

Looks like it worked; a handful of PRs/issues are now starting to get locked.

cevich added a commit to cevich/skopeo that referenced this pull request Aug 28, 2023
Lock discussions on closed PRs and Issues after 90-days of inactivity.

Ref:
     containers/podman#19012
and
     containers/podman#19691
and
     containers/podman#19700

Signed-off-by: Chris Evich <[email protected]>
cevich added a commit to cevich/skopeo that referenced this pull request Aug 28, 2023
Lock discussions on closed PRs and Issues after 90-days of inactivity.

Ref:
     containers/podman#19012
and
     containers/podman#19691
and
     containers/podman#19700

Signed-off-by: Chris Evich <[email protected]>
ranjithrajaram pushed a commit to ranjithrajaram/buildah that referenced this pull request Sep 25, 2023
Lock discussions on closed PRs and Issues after 90-days of inactivity.

Ref:
     containers/podman#19012
and
     containers/podman#19691
and
     containers/podman#19700

Signed-off-by: Chris Evich <[email protected]>
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Nov 22, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. release-note-none
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants