RFC: Close stale PRs - Githubissues

MounirDhahri commented 3 years ago

Proposal:

Close stale PRs after a period of inactivity.

When a PR is inactive for a certain period, we add a label to mark it as 'stale' and a comment to let the author know about it. If someone (the author or someone else) comments in the PR ("still waiting for review" or "work on progress"...), then we remove the stale label.

This is close to the same behavior one gets from Stale but it will mean we will have to implement it in each repo on its own since GitHub organization wide actions is still limited now

Reasoning

Over 50% of open PRs in Eigen, Force, Metaphysics and Volt are stale for a few weeks/months. Most of those PRs are probably abandoned. We want to keep only PRs that are actionable and that might make it to the main branch open and close the stale PRs. PS: Closed PRs won't go anywhere, we still can open them again if needed.

Exceptions:

I am not sure if there should be any exceptions here but we can initially start by applying this only to the repos mentioned above (Eigen, Force, Metaphysics and Volt).
Sometimes, the author of the PR stops working on a PR and gets back to it after a few weeks for example if something else more urgent needs to work on. I suggest we add a custom tag #disablestalecheck to avoid having Danger close the PR

Resolution:

Create a poll in the dev channel to ask for the duration of inactivity we want to set
Add the new rule to Danger

fclesio commented 3 years ago

+1

I think the poll would be great, but maybe we can raise awareness related to the fact that branching itself it's a bit of an antipattern in terms of Continous Integration/Delivery but a necessary pain, having abandoned/Long-lived PRs it's even worse because several issues as:

(i) delayed integration,
(ii) contextual and semantic conflicts, and
(iii) risky deployments since we do not know what we do not know about the side-effects/feedbacks against the current codebase.

The main point that I want to make here it's that we, Artsy Engineers, should be a bit more mindful in terms of PR backlog hygiene.

By the way, thanks for the RFC.

jacobherrington commented 2 years ago

+1 as well!

Closing a PR is not dangerous or permanent in any way, but is a clear indication that something has been handled (merged, deprioritized, abandoned, etc).

There is no reason to keep visual and mental clutter around in projects when we can simply re-open anything that is closed by accident or incorrectly.

MounirDhahri commented 2 years ago

Great, thanks to everyone for your feedback. I will solve this RFC then and bring it to life

MounirDhahri commented 2 years ago

Resolution

We decided to do it

Level of Support

2: Positive feedback.

Next Steps

We will implement it. I will make a poll in the #dev channel to discuss with folks the duration after which we can consider a PR stale.

Exceptions

We will be only implementing this in Eigen, Force, Metaphysics, and Volt then probably enable this for other repos.

artsy / peril-settings

RFC: Close stale PRs #189