A non-core approval changes the label from "awaiting review" to "awaiting core review". I find this useful, and occasionally filter on "awaiting core review" because it indicates that at least one other person found the PR sound / interesting. I would not like this to change - I think high quality reviews from non-core contributors are valuable for the project and are a very quick way for the contributor to get the right kind of attention from core devs.
If people spam the approvals (i.e., approve PRs without reviewing them) then the distinction between the labels becomes meaningless, of course. Though I do wonder what the motivation for doing that repeatedly would be. My basic assumption is that people usually try not to make fools of themselves. Note that contributors can still approve a perfect PR - a quality review in this case would include a brief comment indicating that you understand the change and perhaps why you find it valuable. On the matter of spammy PRs - I agree with both Rupert and Ethan (F): trivial PRs can add value, and a large number of them can be annoying. You can add value and be annoying at the same time (my triage work on b.p.o is probably in this category. Thankfully it's clear I'm not after "triage points", because there aren't any). At the end of the day, it doesn't really matter what a contributor's motivation is - it's up to the core devs to decide which PRs are valuable enough to merge, or even to review, and who they enjoy working with. These things tend to sort themselves out on their own. I don't think we need to restrict access for non-core contributors compared to the status quo. What I do think we need is to make it easier for core devs to close issues and PRs. We have a huge pile of open issues and PRs, some of which we know will never happen (stale or otherwise) and we don't close them because it's an unpleasant task to let someone down, and sometimes they argue, and we're volunteers and why should we bother with this emotional labour. Through triage I found many 6 year old issues that, once I refreshed and perhaps put an 'easy' label on, got fixed. The useless issues we don't want to close are obscuring the ones we can and should fix. I'm sure this has been discussed before. My only idea is that we formalize some guidelines/criteria for closing old issues and PRs that make it more of a joint decision of the core devs and less of a personal issue between the core dev and the contributor. I would not suggest a blanket 'close issues/PRs with no activity for X months', as I said I found useful 6 year old issues. It has to be more sophisticated than that. In summary - my view is that rather than making contributors contribute less, we should be more effective in reviewing the contributions, including rejecting those that should be rejected. On Sun, Jan 30, 2022 at 8:06 AM Ethan Smith <et...@ethanhs.me> wrote: > > > On Sat, Jan 29, 2022 at 7:38 PM Inada Naoki <songofaca...@gmail.com> > wrote: > >> On Sun, Jan 30, 2022 at 12:03 PM Ethan Smith <et...@ethanhs.me> wrote: >> > >> > As a non-committer, I want to make a plea for non-committer approval >> reviews, or at least that they have a place. When asked how outsiders can >> contribute I frequently see "review open PRs" as a suggested way of >> contributing to CPython. Not being able to approve PRs that are good would >> be a barrier to those contributions. >> > >> > Furthermore, I am collaborating with a couple of core devs, it would >> make collaboration more difficult if I couldn't review their work and say >> that I thought the changes looked good. >> > >> >> You can still write a review comment, including "LGTM". > > > Fair enough, I suppose commenting with "LGTM" works just as well. > > >> What you can >> not is labeling PR as "Approved." >> So I don't think it makes collaboration difficult. >> > By preventing approval from others, we can easily find PRs approved >> from core-devs or triage members but not merged yet. >> > >> > I know "drive by approvals" are annoying but I think it is >> unfortunately part of open source projects. >> > >> >> Sorry, but I don't think so. >> > > Well, if you disallow drive-by approvals, you will still get drive-by LGTM > comments. People seem to believe that adding their approval will expedite a > PR merge, for some reason (or want to bump a stale PR in hopes of a quicker > merge). > > >> >> -- >> Inada Naoki <songofaca...@gmail.com> >> > _______________________________________________ > Python-Dev mailing list -- python-dev@python.org > To unsubscribe send an email to python-dev-le...@python.org > https://mail.python.org/mailman3/lists/python-dev.python.org/ > Message archived at > https://mail.python.org/archives/list/python-dev@python.org/message/QJ5QBB4XCFU3DFSOFBRUJB5XC4UMX7TK/ > Code of Conduct: http://python.org/psf/codeofconduct/ >
_______________________________________________ Python-Dev mailing list -- python-dev@python.org To unsubscribe send an email to python-dev-le...@python.org https://mail.python.org/mailman3/lists/python-dev.python.org/ Message archived at https://mail.python.org/archives/list/python-dev@python.org/message/VVYK5RTONJLKIEBJO7VANNXFGTICRFRR/ Code of Conduct: http://python.org/psf/codeofconduct/
