On Tue, Nov 27, 2018 at 5:13 PM Jordan Justen <jordan.l.jus...@intel.com> wrote:
>
> This documents a mechanism for using GitLab Merge Requests as an
> optional, secondary way to get code reviews for patchsets.
>
> We still require all patches to be emailed.
>
> Aside from the potential usage for code review comments, it might also
> help reviewers to find unmerged patchsets which need review. (Assuming
> it doesn't fall into the same fate of patchwork with hundreds of open
> MRs.)
>
> Signed-off-by: Jordan Justen <jordan.l.jus...@intel.com>
> Cc: Jason Ekstrand <ja...@jlekstrand.net>
> ---
> docs/submittingpatches.html | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html
> index 5d8ba443191..852f28c198a 100644
> --- a/docs/submittingpatches.html
> +++ b/docs/submittingpatches.html
> @@ -24,6 +24,7 @@
> <li><a href="#testing">Testing Patches</a>
> <li><a href="#mailing">Mailing Patches</a>
> <li><a href="#reviewing">Reviewing Patches</a>
> +<li><a href="#gitlab">GitLab Merge Requests</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>
> <li><a href="#backports">Sending backports for the stable branch</a>
> @@ -282,6 +283,30 @@ which tells the patch author that the patch can be
> committed, as long
> as the issues are resolved first.
> </p>
>
> +<h2 id="gitlab">GitLab Merge Requests</h2>
> +
> +<p>
> + <a href="https://gitlab.freedesktop.org/mesa/mesa";>GitLab</a> Merge
> + Requests (MR) can be used as an optional, secondary method of
> + obtaining code review for patches.
> +</p>
> +
> +<ul>
> + <li>All patches should be submitted using email as well
> + <li>Consider including a link to the MR in your email based
> + cover-letter
> + <li>Address code review from both email and the MR
Discussion point: I think attempting to have simultaneous review in
two places is a recipe for wasted time. Strawman: maybe we can only
email the cover letter to the mailing list and include in it a link to
the MR?
> + <li>Add appropriate labels to your MR. For example:
> + <ul>
> + <li>Mesa changes affect all drivers: mesa
> + <li>Hardware vendor specific code: amd, intel, nvidia, etc
> + <li>Driver specific code: anvil, freedreno, i965, iris, radeonsi,
> radv, vc4, etc
> + <li>Other tag examples: gallium, util
> + </ul>
> + <li>Never use the merge button on the GitLab page to push patches
Can we disable this in Gitlab? If the button is there, people *will*
accidentally press it.
> + <li>Close your MR when your patches get pushed!
Gentoo has some automation that scans the git log for "Closes: ..."
and automatically closes the merge requests or bugzilla bugs.
Closes: https://github.com/gentoo/gentoo/pull/7423
Closes: https://bugs.gentoo.org/603294
I'm not sure if it's a custom thing or what (I can find out) but I'd
much prefer to automate things if we can. Just like patchwork, people
*will* forget to close merge requests if it's possible. (And people
currently *do* forget to close bugzilla bugs)
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
