<div dir="ltr"><div>Reviewed-by: Matheus Santana <<a href="mailto:embs@cin.ufpe.br">embs@cin.ufpe.br</a>> </div>It seems a great starting point. </div><div class="gmail_extra"> <div class="gmail_quote">On Mon, Jun 18, 2018 at 10:42 AM, Pekka Paalanen <<a href="mailto:ppaalanen@gmail.com" target="_blank">ppaalanen@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">From: Pekka Paalanen <<a href="mailto:pekka.paalanen@collabora.co.uk">pekka.paalanen@collabora.co.uk</a>> This sets up the standards for patch review, and defines when a patch can be merged. I believe these are the practises we have been using already for a long time, now they are just written down explicitly. It's not an exhaustive list of criteria and likely cannot ever be, but it should give a good idea of what level of review we want to have. It has been written in general terms, so that we can easily apply the same text not just to Wayland, but also Weston and other projects as necessary. This addition is not redundant with <a href="https://wayland.freedesktop.org/reviewing.html" rel="noreferrer" target="_blank">https://wayland.freedesktop.org/reviewing.html</a> . The web page is a friendly introduction and encouragement for people to get involved. The guidelines here are more specific and aimed for people who seek commit rights or maintainership. Signed-off-by: Pekka Paalanen <<a href="mailto:pekka.paalanen@collabora.co.uk">pekka.paalanen@collabora.co.uk</a>> --- CONTRIBUTING.md | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7ad75b8..6e74b6d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -197,5 +197,54 @@ New source code files should specify the MIT Expat license in their boilerplate, as part of the copyright statement. +Review +====== + +All patches, even trivial ones, require at least one positive review +(Reviewed-by). Additionally, if no Reviewed-by's have been given by +people with commit access, there needs to be at least one Acked-by from +someone with commit access. A person with commit access is expected to be +able to evaluate the patch with respect to the project scope and architecture. + +During review, the following matters should be checked: + +- The commit message explains why the change is being made. + +- The code fits the project's scope. + +- The code license is the same MIT licence the project generally uses. + +- Stable ABI or API is not broken. + +- Stable ABI or API additions must be justified by actual use cases, not only +by speculation. They must also be documented, and it is strongly recommended to +include tests excercising the additions in the test suite. + +- The code fits the existing software architecture, e.g. no layering +violations. + +- The code is correct and does not ignore corner-cases. + +- In a patch series, every intermediate step produces correct code as well. + +- The code does what it says in the commit message and changes nothing else. + +- The test suite passes. + +- The code does not depend on API or ABI which has no working free open source +implementation. + +- The code is not dead or untestable. E.g. if there are no free open source +software users for it then it is effectively dead code. + +- The code is written to be easy to understand, or if code cannot be clear +enough on its own there are code comments to explain it. + +- The code is minimal, i.e. prefer refactor and re-use when possible unless +clarity suffers. + +- The code adheres to the style guidelines. + + [git documentation]: <a href="http://git-scm.com/documentation" rel="noreferrer" target="_blank">http://git-scm.com/documentation</a> [notes on commit messages]: <a href="http://who-t.blogspot.de/2009/12/on-commit-messages.html" rel="noreferrer" target="_blank">http://who-t.blogspot.de/2009/12/on-commit-messages.html</a> -- 2.16.4 _______________________________________________ wayland-devel mailing list <a href="mailto:wayland-devel@lists.freedesktop.org">wayland-devel@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/wayland-devel" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/wayland-devel</a> </blockquote></div> </div>