Automatically close all enhancement PRs for code lacking an expert?

[stoneleaf (Ethan Furman)]

@stoneleaf

I added us to the experts file back in August. How, and if, that affects the CODEOWNER file I do not know.

@brettcannon

They are totally separate things due to the current split of GitHub and bugs.python.org. You should probably go into CODEOWNERS and add Rhodri and your details for any files involving cgi and cgitb.

How? I don’t see the file in the repo, and the web page for it doesn’t have an <Edit> button.

[dimaqq (Dima Tisnek)]

Let me a devil’s advocate here:

To reach the status quo faster, Bedevere could remove any file in the repo that doesn’t match CODEOWNERS

If that wouldn’t work, perhaps the proposed rule wouldn’t quite work either.

[Mariatta]

@stoneleaf

How? I don’t see the file in the repo, and the web page for it doesn’t have an button.

Codeowners file is at cpython/.github/CODEOWNERS at main · python/cpython · GitHub

You should see edit button (pencil icon ) if you’re logged in to GitHub.

[aeros (Kyle Stanley)]

@brettcannon

Two, it’s a bit demotivating for both PR authors and those trying to work on PRs. For PR authors it gives a feeling that their PR will never be looked at (which is honestly quite possible). And for those working PRs it gives a feeling of dread and hopelessness that one could keep up with the PR queue.

Unfortunately, this is the current reality of the situation for many PRs in areas without experts, especially more complex ones. I’ve personally been at the receiving end of having PRs go without core review for multiple months (most of us probably have at some point), it can be quite demotivating. Nobody is to blame of course, but the more we can do to improve the experience of contributing, the more contributors Python will attract in the long term.

I’d also expand upon that to say that it can also be discouraging for reviewers as well; investing time in reviewing PRs that are left open indefinitely is not a great prospect.

@brettcannon

So to try and get the PR queue down to a more manageable size, I have a somewhat radical idea: automatically close all enhancement PRs that only touches code that has no experts.

I approve of the core proposal here, but I think we need to make significant efforts to ensure the CODEOWNERS file is as updated/populated as possible, before moving forward with automatically closing PRs:

@brettcannon

Ask core developers to update their expertise in CODEOWNERS

This step is particularly critical. Based on a current comparison of the GitHub CODEOWNERS file and the devguide experts page, there are a number of active core developers that are included on the experts page and not CODEOWNERS. There could be a number of reasons for this, but I think part of it might be a simple lack of awareness. I’ve personally tried to reach out to a few, but with limited success.

One potential solution would be to send out a few periodic announcements in python-committers and the Committers section of Discuss, which briefly describes the purpose of CODEOWNERS, how core devs can add themselves, the need for it (particularly if this proposal is accepted), and the (potential) pending removal of the devguide experts list.

However, I think that a particularly effective way of handling this en masse would be to have a dedicated time slot for updating CODEOWNERS at the PyCon 2020 sprint. The proposal is dependent upon around having an adequately populated CODEOWNERS file, so I think bringing all of the active core developers up to date with it is critical to its success.

Plus, having a well covered CODEOWNERS seems like it would be an important part of the bpo → GitHub migration, so this would help with multiple areas.

@brettcannon

I’m starting the discussion here to keep it initially small and to make sure triagers participate in this discussion as they will be part of the front line in marking PRs as type-enhancement . If people are generally on board I can then take it to python-dev.

I’d be glad to help with the process of marking appropriate issues as type-enhancement. However, I would prefer to see how this proposal plays out first before investing too much time into that, as it wouldn’t accomplish much at the moment (for older PRs).

[aeros (Kyle Stanley)]

@dimaqq

To reach the status quo faster, Bedevere could remove any file in the repo that doesn’t match CODEOWNERS

If that wouldn’t work, perhaps the proposed rule wouldn’t quite work either.

I have to strongly disagree with this comparison; removing existing files and closing PRs that add non-essential enhancements is not at all equivalent. Perhaps it might be if @brettcannon was proposing to close all PRs to any file that doesn’t have a CODEOWNERS match, but that’s not the case. This is specifically limited to enhancement PRs, not other types such as security fixes, bug fixes, test changes, performance improvements, or documentation changes.

[zwol (Zack Weinberg)]

I feel very strongly that PRs should never be closed automatically nor in bulk. It is disrespectful to the time and effort that someone put into writing the PR in the first place. Especially when that person is not a regular contributor to the project, it’s likely to cause them to assume that they are being told to go away and never come back.

“We’re sorry, we don’t have the time or attention to work on this enhancement request in its present state” is a legitimate response to an enhancement request, but some human being should take the time to type out a personalized version of it for each and every PR that’s being shed due to lack of people-power. And the personalization should offer a way forward: would it help if the submitter wrote a patch themselves? Or should they start a thread on a mailing list to work out design details? Or is it a case where the only thing that would help is if someone got paid to work on the larger problem for a while? Or what?

Also, a large backlog of bugs is a symptom of an ongoing project management problem that should not be obscured via mass-closings: as several other people have pointed out, they’re just going to keep piling up. The only long-term fix is to find more people to actually work the bugs (which may involve finding money to pay them). If the bug tracker makes it too hard to find the bugs that you care about in a morass of other stuff, that’s also a symptom of an underlying problem (the bug tracker itself needs improvement) that’s not going to go away if you paint over it.

[stoneleaf (Ethan Furman)]

@Mariatta

You should see edit button (pencil icon ) if you’re logged in to GitHub.

Ah, there it is. Silly me, looking for the word “Edit”. Is this the appropriate place to complain about the younger generation?

[brettcannon]

@zwol

I feel very strongly that PRs should never be closed automatically nor in bulk. It is disrespectful to the time and effort that someone put into writing the PR in the first place.

Also note this can go the other direction as well: if we have a contributors guide which explicitly lays out what we expect from people to consider a PR and they choose to ignore it then it’s a waste of our time to have to point out that they chose to ignore the documentation.

[brettcannon]

So what I have gotten out of this conversation is two things:

1. Updating Bedevere to apply some “no experts” label to PRs would be helpful and be a good way to surface areas being neglected
2. We need to update our set up for listing experts to take into consideration CODEOWNERS

Thanks everyone for the conversation! I’m going to close this topic as both those ideas should be done in different places or separate topics.