Seems like a pretty good sum-up of the discussion to me. On Mon, Dec 3, 2018 at 7:50 PM Jordan Justen <jordan.l.jus...@intel.com> wrote:
> Lots of discussion here. :) I hoped to just dip our toes into the > merge request stream, but the consensus is clearly against my idea. Oh > well. :) > > Here's what I gathered from the discussion: > > == Keep email, allow duplicate MR, my idea == > > I don't think anyone else was in favor of this. Several were against > it. > > == NACK merge requests altogether == > > I did not see this, so that seems promising for merge requests. :) I > guess no one strongly disagrees with trying merge requests. > > == Allow submitter (or driver) to choose to email or MR, but not both == > > Several seemed to argue that this could be a good next step. But, in > all those cases, I think they wanted to use merge requests going > forward. > > It was mentioned that vc4 has moved to github in order to allow github > pull requests to be used. > > == Require all email list, or all merge requests == > > A few argued in favor of this, and seemed to feel pretty strongly > about it. In both cases I believe they wanted to more to merge > requests. :) > > == Next step? == > > So, what do you all think is the best next step after the discussion? > > We could take the smaller step to allow the submitter, or project to > decide which they prefer. It would allow those who prefer email to > continue with email, while trying out gitlab merge requests. But, it > could mean that devs need to check both gitlab and email. > > Or, we could just try to make the jump to MR only. Maybe if I write > that patch someone will finally stand up to try to NACK moving away > from email. :) > > Or, are we a little too early for this discussion, and we should just > drop it for now? > > My opinion would be to try to move a bit slower and allow the > submitter/project to choose for a trial period. (Dylan and Daniel seem > to think this could be really frustrating for devs though.) But, if > everyone seems to think it's reasonable to try to jump straight to > using merge requests exclusively, I can type that version up... > I agree, for two reasons: 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. 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. 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. --Jason > -Jordan > > On 2018-11-27 17:13:38, Jordan Justen 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 > > + <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 > > + <li>Add Reviewed-by tags to your commits and push using git > > + <li>Close your MR when your patches get pushed! > > +</ul> > > > > <h2 id="nominations">Nominating a commit for a stable branch</h2> > > > > -- > > 2.20.0.rc1 > > >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
