As a non-core contributor who has been regularly submitting constructive reviews recently, I would have to disagree with this proposal. However, I do agree with the idea of rejecting non-core approvals without feedback, and doing as much as possible to communicate to contributors that their reviews are valuable primarily for constructive feedback, instead of the approval itself.
There have been a few times where I’ve been able to make suggestions to first-time contributors with regards to the CLA (in case they have to manually update the status, like I did) or patchcheck
process. Having contributors assist with this rather than core devs helps to save valuable time.
With regards to the review process though, I think that it might be beneficial to have a intermediate position within the community similar the “triager” role that specializes in reviewing PRs from from non-core contributors. They could potentially help with assisting first-time contributors, adding labels (such as skip news
and skip issue
for minor ones), and attaching core devs as reviewers to unassigned PRs.
An issue with the existing system might be the automatic label changing from awaiting review
to awaiting core review
. At the moment, the label is changed whenever anyone adds a review to an issue, even if the person reviewing the changes chooses the request changes
option. At the least, this should be fixed so that the label is only changed upon an approval. Another option might be to have that ability reserved to that specific intermediary role, but that’s a bit more long term since that role does not currently exist.
Edit: As for constructive reviews from non-core developers, I feel that this one might serve as a decent example: https://github.com/python/cpython/pull/14593.