<div dir="ltr"><div class="gmail_quote"><div>Seems like a pretty good sum-up of the discussion to me.<br></div><div dir="ltr"><br></div><div dir="ltr">On Mon, Dec 3, 2018 at 7:50 PM Jordan Justen <<a href="mailto:jordan.l.justen@intel.com">jordan.l.justen@intel.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Lots of discussion here. :) I hoped to just dip our toes into the<br>
merge request stream, but the consensus is clearly against my idea. Oh<br>
well. :)<br>
<br>
Here's what I gathered from the discussion:<br>
<br>
== Keep email, allow duplicate MR, my idea ==<br>
<br>
I don't think anyone else was in favor of this. Several were against<br>
it.<br>
<br>
== NACK merge requests altogether ==<br>
<br>
I did not see this, so that seems promising for merge requests. :) I<br>
guess no one strongly disagrees with trying merge requests.<br>
<br>
== Allow submitter (or driver) to choose to email or MR, but not both ==<br>
<br>
Several seemed to argue that this could be a good next step. But, in<br>
all those cases, I think they wanted to use merge requests going<br>
forward.<br>
<br>
It was mentioned that vc4 has moved to github in order to allow github<br>
pull requests to be used.<br>
<br>
== Require all email list, or all merge requests ==<br>
<br>
A few argued in favor of this, and seemed to feel pretty strongly<br>
about it. In both cases I believe they wanted to more to merge<br>
requests. :)<br>
<br>
== Next step? ==<br>
<br>
So, what do you all think is the best next step after the discussion?<br>
<br>
We could take the smaller step to allow the submitter, or project to<br>
decide which they prefer. It would allow those who prefer email to<br>
continue with email, while trying out gitlab merge requests. But, it<br>
could mean that devs need to check both gitlab and email.<br>
<br>
Or, we could just try to make the jump to MR only. Maybe if I write<br>
that patch someone will finally stand up to try to NACK moving away<br>
from email. :)<br>
<br>
Or, are we a little too early for this discussion, and we should just<br>
drop it for now?<br>
<br>
My opinion would be to try to move a bit slower and allow the<br>
submitter/project to choose for a trial period. (Dylan and Daniel seem<br>
to think this could be really frustrating for devs though.) But, if<br>
everyone seems to think it's reasonable to try to jump straight to<br>
using merge requests exclusively, I can type that version up...<br>
</blockquote><div><br></div><div>I agree, for two reasons:</div><div><br></div><div> 1) Hard switches are hard on everyone; some people have outstanding work that is mid-review and telling them to move it to a MR immediately may be rough.  It can also be a big workflow break and I think it's good to let people change their workflow at their own pace for now.<br></div><div><br></div><div> 2) It gives us a way out if things go sideways.  I don't expect it'll be too bad but there will be issues.  Some people may prefer to keep sending e-mails until certain issues are addressed in some way.</div><div><br></div><div>That said, given the over-all positive response towards MRs, I expect the list will become a ghost town in no time and the full transition will happen all by itself.<br></div><div><br></div><div>--Jason<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
-Jordan<br>
<br>
On 2018-11-27 17:13:38, Jordan Justen wrote:<br>
> This documents a mechanism for using GitLab Merge Requests as an<br>
> optional, secondary way to get code reviews for patchsets.<br>
> <br>
> We still require all patches to be emailed.<br>
> <br>
> Aside from the potential usage for code review comments, it might also<br>
> help reviewers to find unmerged patchsets which need review. (Assuming<br>
> it doesn't fall into the same fate of patchwork with hundreds of open<br>
> MRs.)<br>
> <br>
> Signed-off-by: Jordan Justen <<a href="mailto:jordan.l.justen@intel.com" target="_blank">jordan.l.justen@intel.com</a>><br>
> Cc: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>><br>
> ---<br>
>  docs/submittingpatches.html | 25 +++++++++++++++++++++++++<br>
>  1 file changed, 25 insertions(+)<br>
> <br>
> diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html<br>
> index 5d8ba443191..852f28c198a 100644<br>
> --- a/docs/submittingpatches.html<br>
> +++ b/docs/submittingpatches.html<br>
> @@ -24,6 +24,7 @@<br>
>  <li><a href="#testing">Testing Patches</a><br>
>  <li><a href="#mailing">Mailing Patches</a><br>
>  <li><a href="#reviewing">Reviewing Patches</a><br>
> +<li><a href="#gitlab">GitLab Merge Requests</a><br>
>  <li><a href="#nominations">Nominating a commit for a stable branch</a><br>
>  <li><a href="#criteria">Criteria for accepting patches to the stable branch</a><br>
>  <li><a href="#backports">Sending backports for the stable branch</a><br>
> @@ -282,6 +283,30 @@ which tells the patch author that the patch can be committed, as long<br>
>  as the issues are resolved first.<br>
>  </p><br>
>  <br>
> +<h2 id="gitlab">GitLab Merge Requests</h2><br>
> +<br>
> +<p><br>
> +  <a href="<a href="https://gitlab.freedesktop.org/mesa/mesa" rel="noreferrer" target="_blank">https://gitlab.freedesktop.org/mesa/mesa</a>">GitLab</a> Merge<br>
> +  Requests (MR) can be used as an optional, secondary method of<br>
> +  obtaining code review for patches.<br>
> +</p><br>
> +<br>
> +<ul><br>
> +  <li>All patches should be submitted using email as well<br>
> +  <li>Consider including a link to the MR in your email based<br>
> +    cover-letter<br>
> +  <li>Address code review from both email and the MR<br>
> +  <li>Add appropriate labels to your MR. For example:<br>
> +    <ul><br>
> +      <li>Mesa changes affect all drivers: mesa<br>
> +      <li>Hardware vendor specific code: amd, intel, nvidia, etc<br>
> +      <li>Driver specific code: anvil, freedreno, i965, iris, radeonsi, radv, vc4, etc<br>
> +      <li>Other tag examples: gallium, util<br>
> +    </ul><br>
> +  <li>Never use the merge button on the GitLab page to push patches<br>
> +  <li>Add Reviewed-by tags to your commits and push using git<br>
> +  <li>Close your MR when your patches get pushed!<br>
> +</ul><br>
>  <br>
>  <h2 id="nominations">Nominating a commit for a stable branch</h2><br>
>  <br>
> -- <br>
> 2.20.0.rc1<br>
> <br>
</blockquote></div></div>