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

KAFKA-15073 Close stale PRs [2/n] #17166

Merged
merged 5 commits into from
Sep 11, 2024
Merged

Conversation

mumrah
Copy link
Member

@mumrah mumrah commented Sep 11, 2024

As a follow-up of #13827, this patch will allow the stale Github Action to actually close old PRs.

@mumrah mumrah added the build Gradle build or GitHub Actions label Sep 11, 2024
Copy link
Member

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

@mumrah nice patch! 1.1k pending PRs is a bit crazy ...

.github/workflows/stale.yml Show resolved Hide resolved
@mumrah mumrah requested review from jlprat and mimaison September 11, 2024 18:20
Copy link
Contributor

@jlprat jlprat left a comment

Choose a reason for hiding this comment

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

I left a comment, let me know what you think

days-before-stale: 90
days-before-close: -1
days-before-close: 120
Copy link
Contributor

Choose a reason for hiding this comment

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

I know it's probably not doable, but it would be great to close only the PRs where the contributor dropped the ball, and leave the ones waiting for us maintainers to review open.
With the closing message we leave the door open for the contributor to reopen, maybe we can be more explicit stating that it's totally fine to reopen if the PR is under this case

Copy link
Member Author

Choose a reason for hiding this comment

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

From the actions/stale docs

It means that any updates made, or any comments added to the issues or to the pull requests will restart the counter of days before marking as stale.

So, if the contributor adds a comment, it will reset the counter for being stale. I'll clarify this in the stale message.

There is probably some additional automation we can look into for raising our attention to do reviews.

Copy link
Contributor

@jlprat jlprat left a comment

Choose a reason for hiding this comment

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

LGTM, I think leaving 30 days for the contributor to act is an acceptable amount of time. Also they can reopen if they feel the closing was wrong

@chia7712
Copy link
Member

I think leaving 30 days for the contributor to act is an acceptable amount of time. Also they can reopen if they feel the closing was wrong

If we want to address "30 days", the value of "days-before-close" should be "30" rather than "120". see docs: https://github.com/actions/stale?tab=readme-ov-file#days-before-close

Since adding the stale label will alter the last update date, we can calculate the number of days from this date.

@mumrah
Copy link
Member Author

mumrah commented Sep 11, 2024

Good catch, @chia7712 👍 -- will adjust accordingly.

@jlprat
Copy link
Contributor

jlprat commented Sep 11, 2024

I think leaving 30 days for the contributor to act is an acceptable amount of time. Also they can reopen if they feel the closing was wrong

If we want to address "30 days", the value of "days-before-close" should be "30" rather than "120". see docs: https://github.com/actions/stale?tab=readme-ov-file#days-before-close

Since adding the stale label will alter the last update date, we can calculate the number of days from this date.

Great catch! I understood it as total time. But you are right adding a comment and label will modify the last state for the PR

Copy link
Contributor

@jlprat jlprat left a comment

Choose a reason for hiding this comment

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

Re LGTM for clarity.
Thanks @mumrah

@mumrah mumrah merged commit ad19d29 into apache:trunk Sep 11, 2024
5 of 7 checks passed
@mumrah mumrah deleted the enable-close-stale-prs branch September 11, 2024 19:10
@mumrah
Copy link
Member Author

mumrah commented Sep 11, 2024

Looks like its working as expected.

I did a manual run here https://github.com/apache/kafka/actions/runs/10818124385/job/30013048044

The 25 oldest PRs are now labelled stale https://github.com/apache/kafka/pulls?q=is%3Aopen+label%3Astale+sort%3Acreated-asc

tedyu pushed a commit to tedyu/kafka that referenced this pull request Jan 6, 2025
As a follow-up of apache#13827, this patch updates the stale PR workflow to automatically close PRs that have not had activity in 120 days

Reviewers: Chia-Ping Tsai <[email protected]>, Josep Prat <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Gradle build or GitHub Actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants