Hello,
I think this is ok
If someone wants their PR merged they have to work on it otherwise it is
forgotten.
Development is *never* that urgent, delays for merging are normal and
acceptable.
Sebastien
On 10/2/25 13:24, Alan C. Assis 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
