<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Tue, Dec 11, 2018 at 8:15 PM Ian Romanick <<a href="mailto:idr@freedesktop.org">idr@freedesktop.org</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">It's fairly common for Mesa developers to have several patch series on the mailing list at a time. I believe most people will author these as a continuous stream with either implicit dependencies (i.e., commit messages in the second series with shader-db results that are impacted by the first series) or explicit dependencies (i.e., second series uses interfaces added or changed by the first). When I do this, I still like to send the series out a separate sets of patches that accomplish separate, logical tasks. We can't be the only project where people work like this, so what is the common practice in other projects? I've thought of several possible solutions, but each seems fatally flawed. - A single, possibly giant, MR defeats the ability to have separate logical work units. - Splitting into multiple MRs based from master may not be practical or possible without including some patches in both series. - Waiting to send the second series out may increase the lag in the review process or lead to the submitter forgetting to submit. :) </blockquote><div> </div><div>What I would do would be to create a series of MRs that are each based on the one before. It gets a little weird because they get longer and longer but if you have some commentary in the MR header saying that it's based on another one, it shouldn't be so bad. If someone is doing patch-by-patch review, they can easily enough just comment on the patches unique to that MR.</div><div> </div><div>Sadly, that suggestion won't work quite the way you want as I don't think GitLab has a concept of dependent MRs. Maybe that's another thing to add to the feature request list? </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> On 12/5/18 3:32 PM, 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 <<a href="mailto:jordan.l.justen@intel.com" target="_blank">jordan.l.justen@intel.com</a>> > --- > 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. > > > > -<h2 id="mailing">Mailing Patches</h2> > +<h2 id="submit">Submitting Patches</h2> > > > -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. > + > + > +<h3 id="mailing">Mailing Patches</h3> > + > + > +Patches may be sent to the mesa-dev mailing list for review: > <a href="<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a>"> > <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a></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.) > > > +<h3 id="merge-request">GitLab Merge Requests</h3> > + > + > + <a href="<a href="https://gitlab.freedesktop.org/mesa/mesa" rel="noreferrer" target="_blank">https://gitlab.freedesktop.org/mesa/mesa</a>">GitLab</a> Merge > + Requests (MR) can also be used to submit patches for Mesa. > + > + > + > + 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. > + > + > + 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> > + > + > + If you revise your patches based on code review and push an update > + to your branch, you should maintain a clean 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. > + > + > + 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. > + > + > + 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> > + > + > <h2 id="reviewing">Reviewing Patches</h2> > > + > + To participate in code review, you should monitor the > + <a href="<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a>"> > + mesa-dev</a> email list and the GitLab > + Mesa <a href="<a href="https://gitlab.freedesktop.org/mesa/mesa/merge_requests" rel="noreferrer" target="_blank">https://gitlab.freedesktop.org/mesa/mesa/merge_requests</a>">Merge > + Requests</a> page. > + > + > > When you've reviewed a patch on the mailing list, please be unambiguous > about your review. That is, state either > _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div></div>