Automatically marking stale PRs as such

Hello everyone!

I would like Django to start using some form of automatic checker for stale PRs. I did some investigation and there is a GitHub Action that would provide this functionality for us:

Any objections to try it out and evaluate how that works for us?

Thanks!
Natalia.

I’d kind of say it’s up to you and Mariusz… — it’s you two that have to run the repo.

I’m sceptical of the value myself. When it was added to the django/djangoproject.com repo I just had to more or less fight it saying “Hang-on, don’t close that!” (Sure we did close some, but I could have just sorted by least recently updated and looked at the first page for equal results.) (Part of the issue there may have been that the defaults are too aggressive for an open source project.)

I agree that we should tweak the default, particularly I think that the 60 days for automatically adding the “Stale” label is a sane default but closing 7 days after that is too aggressive. I would suggest we set 30 days after setting the label, to allow for holidays/DjangoCons/etc to happen in between and not have automatic closures.

I think it’s rather unfriendly to use bots, especially on PRs that didn’t even receive a response from a human. I raised a previous object to using it for djangoproject.com. I would say it takes a minute or two to look at a PR and evaluate what should be done. Sometimes work takes a while, with periods of inactivity, and there is value in keeping old PRs open so we don’t create duplicates with feedback and history split between locations. If having a lot of old PRs open bothers you, just ignore them. :wink: (But seriously, what would it accomplish? Do you feel overwhelmed by the backlog?)

I share your opinion

This is a valid point, and I appreciate you bringing it up. I was contemplating a scenario where PRs would be marked as “Stale” if a reviewer requested feedback from the author and received no response within a specified timeframe, for example (currently these are the many comments “Hello @author, would you have time to keep working on this?”).

I read this post and while I agree with your rationale for (not) automatic closing old issues (since they may very well be still valid), I also agree with Sarah in that the automatic flagging of Stale PRs makes more sense, which is what I’m proposing in this post (“There are configurations around stale PR closing which I think makes more sense in general … , as other people might be reluctant to work on something if there is an open PR and it prompts someone to decide if they want to come back to this if they have just forgotten for example.”).

I don’t agree with this sentence; it feels like an oversimplification of the process. At least to me, this takes around 15-30 minutes per PR. The process involves reading the linked ticket and its comments, the PR history, and general PR comments. Then, reviewing the non-resolved inline comments and identifying who is who (authors vs reviewers). This also includes assessing whether there is a pending need for agreement or if there was an agreement that was not pursued. The next step is the evaluation of whether whatever is left is worth completing using fellow’s time or if we should ask the author whether they have time/interest in pushing the work to the final line. I usually opt for the latter to encourage community participation. However, this means I need to remember 15 or 30 days later to come back to these PRs and close them (or complete the work if it makes sense).

I don’t disagree with this sentence but from my observation of current practices (and this is where I may be over simplifying), it seems that is preferred to be a bit more aggressive with closing PRs, so (new) contributors can more clearly see where there is work to do and can pick up where others left (it seems to be a bit easier for people to build on an existing stale PR than starting from scratch).

It doesn’t bother me, and this isn’t a matter of personal taste or preference. My intention was to contribute to the overall health of the Django project from the perspective of my role as a fellow. Additionally, given that the fellows invest time in pinging and closing stale PRs, I thought we could allocate those minutes to other, less automatable activities, such as reviews of active PRs.

I don’t personally experience this, but I believe others might. New contributors, eager to help as we encourage in the Contributing guide, may feel lost and overwhelmed when confronted with the sizable open PRs queue. Questions may arise on where to start – should it be the oldest, the least recently updated, or the smaller ones? They might also wonder “Why are there so many open PRs? Is this a community that doesn’t pay attention to contributions?”

In any case, your message helped me realize that I could tailor my proposal a bit more for the sake of clarity. If the initial interaction with the bot is an issue (and I agree it is), we could tweak the process as follows:

  1. Create a new label to flag PRs that may be candidate for closing/cleaning up if there is no activity in a given amount of time (it could be 30 days). This label could be “Stale” or whatever we consider best.
  2. Do not have a bot adding this flag automatically, but have “humans” setting it. So anyone willing to take the time to review the PR and ticket history could add a summary comment in the PR and set the label, and if no activity occurs in 30 days, the bot does close it.

Thanks! Natalia.

This isn’t a bad idea at all. In general folks can’t close a PR—they don’t have permission—so even if they take a look, and think “this is outdated”, it still needs a Fellow to follow up and close it. If a comment, like the buildbot ones could apply a label, then that could be acted on (automatically or not), which brings PRs closer in-line with Trac (where anyone can make edits, and help triage).

I’m much more sympathetic to something along those lines than just the Stale bot, which, repeating myself :woman_facepalming:, it a bit blunt

1 Like

TBH, I don’t like bots (unpopular opinion) and they seem unnecessary at our scale (~150 open PR). I offen struggle with closing old PRs because I have a gut feeling that something more needs to be said (more feedback etc.) and I don’t want to be forced to make decision faster because a soulless bot will push me.

My 2¢.

2 Likes

Thank you everyone for their feedback.

I’ve been thinking about this and I acknowledge the mentioned counterpoints, thanks for raising those.
I still think that some form of automation would be beneficial, what would you think about a reminder bot? That way people could implement their own flow, for example a flow that I would find very useful is:

  1. review a PR
  2. notice issue X and Y that needs addressing, post comment
  3. set a reminder to check back in a week or so

Then, there a are a few alternate derived flows. Sometimes I then get immediate responses with fixes, which I’ll review/evaluate/comment back, or I’ll get responses saying “I’ll be slow because this and that” so I could reschedule the reminder to happen in a month or so. Eventually this could lead to PR closing (by a human) but with the benefit that “time accounting” would be done automatically.

Or, I may want to re-check a PR in 3 or 4 weeks because there is a release in between, or a conference, or similar.

I’ve found a few GH actions that act as reminders creator/reminder poster, though the process of “resetting a reminder” may need to be done manually (removing a label).

Would this be of interest of anyone else? I know it’d definitely help my flow but I wouldn’t want to push this setup if it’s only appealing to me.

Thank you!
Natalia.

Hello everyone,

I’m keen to push some of this forward. Concretely, what I would like to do is:

  1. have the ability to manually set reminders in PRs (available for everyone)
  2. have automatic messages being posted in PRs about two things:
    2.1. if a PR has not seen activity in a given period of time, remind the author to check if the proper Trac flags have been set (I have been doing this manually recently and I think it’s a waste of resources have a human/fellow doing this)
    2.2. after a period of author inactivity, confirm if the author has time/desire to keep working on the feature

In all cases, no bot would be closing a PR, always a human would (when necessary).

My main aim with this (and similar ideas) is to make it easier for reviewers/fellows to manage PRs (and their time!) efficiently. It’s also about helping contributors follow the correct process without bombarding them in lots of links to docs. In my view, sending contributors to read a lot of docs can be less user-friendly than giving them prompt and clear messages about the next steps. This helps set everyone’s expectations correctly, IMHO.

Natalia.

PS: When searching for existing options, I found this issue which describes a flow very similar to what I’d have in mind.

My ideal django helper bot would be a weekly job that checks django PRs that haven’t had activity (commits or comments) in 2 months and are not in the review queue (using the PR title to get the ticket number and skipping ones that don’t have this). Then it has an automated message saying something like: “Are you still working on this? This isn’t in the review queue - if you’re waiting for a review please check that the ticket has ‘patch needs improvement’, ‘needs tests’, ‘needs docs’ set to False and ‘has patch’ set to True.” and it labels the PR something like ‘stale’.
Then anyone in the community can check the stale PRs that have had no updates either on Trac or the PR after maybe 2 weeks and close with a message that they can reopen. Or, if it’s clear that they are actually waiting for a review but forgot to update Trac, we do this.

As a meta comment, if the automation will:

  • help the fellow workload
  • is polite and respectful
  • is specifically designed for/tailored to Django contributing process

Then I think we should do it, it can always be improved based on feedback or reverted if it causes problems. :woman_shrugging:

What you describe here sounds quite reasonable to me @nessita.

It’s also the case that you’re in charge (for some it’s not a dictatorship value of that) It’s you that has to run the repo now. So if you want to give it a go, I’m happy to put a +1 here.

Good hustle! :gift:

I’m giving the add/get reminders a go, I have proposed two new GH actions in this PR: Added GitHub Actions to create and check for reminders in pull requests. by nessita · Pull Request #17852 · django/django · GitHub

1 Like