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
