[PATCH wayland 1/2] contributing: add review guidelines

[Matheus Santana]

Reviewed-by: Matheus Santana <embs at cin.ufpe.br>

It seems a great starting point.

On Mon, Jun 18, 2018 at 10:42 AM, Pekka Paalanen <ppaalanen at gmail.com>
wrote:

> From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
>
> 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
> https://wayland.freedesktop.org/reviewing.html .
>
> 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 <pekka.paalanen at collabora.co.uk>
> ---
>  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]: http://git-scm.com/documentation
>  [notes on commit messages]: http://who-t.blogspot.de/2009/
> 12/on-commit-messages.html
> --
> 2.16.4
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180618/c2adb676/attachment.html>