On Mon, Jul 1, 2024 at 4:34 PM Philippe Mathieu-Daudé <phi...@linaro.org> wrote: > Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org>
In principle, a Reviewed-by tag is just stating that you don't know of any issues that would prevent the patch being included. However, as a frequent participant to the project, your Reviewed-by tag carries some weight and, to some extent, it is also a statement that you understand the area being modified. A Reviewed-by from an experienced contributor may even imply that you could take the patch in one of your pull requests. (*) That makes it even more important to understand the area. I would expect that anyone with an understanding of command line parsing would know 1) what -accel kvm -accel tcg does, and 2) what .merge_lists does; and this would be enough to flag an issue preventing the patch from being included. To be clear, I don't expect reviews to be perfect. But in this case I'm speaking up because the patch is literally a one line declarative change, and the only way to say "I've reviewed it" is by understanding the deeper effects of that line. Also, I think it's fair that the submitter didn't spot the problem; it's okay to send out broken patches, that's part of the learning experience. :) Paolo (*) as opposed to Acked-by, where your review probably has been more conceptual than technical, and that you don't really want to take the patch in a pull request. Paolo
