[Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)

Thu Dec 6 21:57:09 UTC 2018

On 06.12.18 00:32, Jordan Justen wrote:
> This documents a process for using GitLab Merge Requests as an second
> way to submit code changes for Mesa. Only one of the two methods is
> allowed for each patch series.
> 
> We will *not* require all patches to be emailed. Some code changes may
> be reviewed and merged without any discussion on the mesa-dev email
> list.
> 
> v2:
>   * No longer require email. Allow submitter to choose email or a
>     GitLab merge request.
>   * Various feedback from Brian, Daniel, Dylan, Eric, Erik, Jason,
>     Matt, Michel and Rob.
> 
> Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
> ---
>   docs/submittingpatches.html | 76 ++++++++++++++++++++++++++++++++++---
>   1 file changed, 71 insertions(+), 5 deletions(-)
> 
> diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html
> index 92d954a2d09..21175988d0b 100644
> --- a/docs/submittingpatches.html
> +++ b/docs/submittingpatches.html
> @@ -21,7 +21,7 @@
>   <li><a href="#guidelines">Basic guidelines</a>
>   <li><a href="#formatting">Patch formatting</a>
>   <li><a href="#testing">Testing Patches</a>
> -<li><a href="#mailing">Mailing Patches</a>
> +<li><a href="#submit">Submitting Patches</a>
>   <li><a href="#reviewing">Reviewing Patches</a>
>   <li><a href="#nominations">Nominating a commit for a stable branch</a>
>   <li><a href="#criteria">Criteria for accepting patches to the stable branch</a>
> @@ -42,8 +42,10 @@ components.
>   <code>git bisect</code>.)
>   <li>Patches should be properly <a href="#formatting">formatted</a>.
>   <li>Patches should be sufficiently <a href="#testing">tested</a> before submitting.
> -<li>Patches should be submitted to <a href="#mailing">mesa-dev</a>
> -for <a href="#reviewing">review</a> using <code>git send-email</code>.
> +<li>Patches should be <a href="#submit">submitted</a>
> +to <a href="#mailing">mesa-dev</a> or with
> +a <a href="#merge-request">merge request</a>
> +for <a href="#reviewing">review</a>.
>   
>   </ul>
>   
> @@ -180,10 +182,19 @@ run.
>   </p>
>   
>   
> -<h2 id="mailing">Mailing Patches</h2>
> +<h2 id="submit">Submitting Patches</h2>
>   
>   <p>
> -Patches should be sent to the mesa-dev mailing list for review:
> +Patches may be submitted to the Mesa project by
> +<a href="#mailing">email</a> or with a
> +GitLab <a href="#merge-request">merge request</a>. To prevent
> +duplicate code review, only use one method to submit your changes.
> +</p>
> +
> +<h3 id="mailing">Mailing Patches</h3>
> +
> +<p>
> +Patches may be sent to the mesa-dev mailing list for review:
>   <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev">
>   mesa-dev at lists.freedesktop.org</a>.
>   When submitting a patch make sure to use
> @@ -217,8 +228,63 @@ disabled before sending your patches. (Note that you may need to contact
>   your email administrator for this.)
>   </p>
>   
> +<h3 id="merge-request">GitLab Merge Requests</h3>
> +
> +<p>
> +  <a href="https://gitlab.freedesktop.org/mesa/mesa">GitLab</a> Merge
> +  Requests (MR) can also be used to submit patches for Mesa.
> +</p>
> +
> +<p>
> +  If the MR may have interest for most of the Mesa community, you can
> +  send an email to the mesa-dev email list including a link to the MR.
> +  Don't send the patch to mesa-dev, just the MR link.
> +</p>
> +<p>
> +  Add labels to your MR to help reviewers find it. For example:
> +  <ul>
> +    <li>Mesa changes affecting all drivers: mesa
> +    <li>Hardware vendor specific code: amd, intel, nvidia, ...
> +    <li>Driver specific code: anvil, freedreno, i965, iris, radeonsi,
> +      radv, vc4, ...
> +    <li>Other tag examples: gallium, util
> +  </ul>
> +</p>
> +<p>
> +  If you revise your patches based on code review and push an update
> +  to your branch, you should maintain a <strong>clean</strong> history
> +  in your patches. There should not be "fixup" patches in the history.
> +  The series should be buildable and functional after every commit
> +  whenever you push the branch.
> +</p>
> +<p>
> +  It is your responsibility to keep the MR alive and making progress,
> +  as there are no guarantees that a Mesa dev will independently take
> +  interest in it.
> +</p>
> +<p>
> +  Some other notes:
> +  <ul>
> +    <li>Make changes and update your branch based on feedback
> +    <li>Old, stale MR may be closed, but you can reopen it if you
> +      still want to pursue the changes
> +    <li>You should periodically check to see if your MR needs to be
> +      rebased
> +    <li>Make sure your MR is closed if your patches get pushed outside
> +      of GitLab
> +  </ul>
> +</p>
> +
>   <h2 id="reviewing">Reviewing Patches</h2>
>   
> +<p>
> +  To participate in code review, you should monitor the
> +  <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev">
> +  mesa-dev</a> email list and the GitLab
> +  Mesa <a href="https://gitlab.freedesktop.org/mesa/mesa/merge_requests">Merge
> +  Requests</a> page.

This link is broken.

What's the best way to get a feel for how the review process would work 
in practice?

Cheers,
Nicolai

> +</p>
> +
>   <p>
>   When you've reviewed a patch on the mailing list, please be unambiguous
>   about your review.  That is, state either
> 

-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.