I'm not sure if that's possible on GitHub. I agree that it's important to be able to unblock PRs if a reviewer becomes inactive, so I also don't really want to remove this feature. I just think it needs to be really clear to reviewers so that they don't dismiss reviews unless there someone inactive.
Matteo On Thu, Oct 2, 2025, 7:48 AM Alan C. Assis <[email protected]> wrote: > I think maybe it could be possible to configure github to not allow other > committer to dismiss the Change Request, but it has a drawback/danger: > > Imagine someone added a Request Change and never came back to remove the > change request, the PR could be stuck there forever. > > BR, > > Alan > > On Wed, Oct 1, 2025 at 2:13 PM Matteo Golin <[email protected]> > wrote: > > > Hello everyone, > > > > I have noticed a large number of PRs have been getting merged, even > though > > they do not necessarily meet the contributing guidelines that we had > voted > > on back in March: > > https://www.mail-archive.com/[email protected]/msg12795.html > > > > The major issue I have spotted is PRs with no proof of testing, no logs > and > > no information about the test cases used being merged. I have seen that > > some reviewers comment on this lack of information, but most of us seem > to > > just use the "comment" feature on reviews. I think that if you are > > requesting that more information be included in the PR description, or > > really any changes, you should use the "request changes" option, which > > prevents the PR from being merged until the changes are made. However, > > there have unfortunately been cases where the review comments have been > > ignored and the PR merged anyways, and even cases where "requests for > > changes" are dismissed and the PR merged. > > > > I think we need to be reminded that we voted in favour of a zero-trust > > approach to user/author testing, and that it is the author's > responsibility > > to provide build and runtime logs for real world hardware. We also voted > > for breaking changes to be validated on various real-world devices with > > runtime logs, and 4 independent approvals on the PR. QEMU is not > considered > > valid for this explicitly. Obviously there are cases where some testing > may > > not be required (i.e. fixing typos in comment strings), but the user > should > > note that in their PR's testing section. > > > > I propose that we include some kind of automation (similar to how Lup had > > the auto-PR reviewer a while back) which just comments these contribution > > guidelines on the PRs themselves. I don't really know how we can be more > > explicit towards contributors (although suggestions are welcome) since > our > > guidelines are very clear and the PR template itself states the > > requirements, which are often being ignored. I am mainly trying to > resolve > > reviewers forgetting about these guidelines so we can prevent these > merges > > of non-compliant PRs from taking place. Usually it is just a small change > > required by the author (re-running their test to capture the log output > and > > include it). I think that it's very important that we tackle this issue, > > because we've had multiple complaints on this mailing list now about > > regressions being introduced and PRs being merged with too much haste. > > > > I am looking for input from the community about what we can do to prevent > > these kinds of PRs from getting merged and facilitate the review process > so > > that reviewers are actually following the agreed upon guidelines (I know > > that there are sometimes many to remember). > > > > Best, > > Matteo > > >
