<div dir="ltr"><div dir="ltr">On Wed, Oct 6, 2021 at 1:27 PM Bas Nieuwenhuizen <<a href="mailto:bas@basnieuwenhuizen.nl">bas@basnieuwenhuizen.nl</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Wed, Oct 6, 2021 at 7:07 PM Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>> wrote:<br>
><br>
> On Wed, Oct 6, 2021 at 11:24 AM Emma Anholt <<a href="mailto:emma@anholt.net" target="_blank">emma@anholt.net</a>> wrote:<br>
> ><br>
> > On Wed, Oct 6, 2021 at 9:20 AM Mike Blumenkrantz<br>
> > <<a href="mailto:michael.blumenkrantz@gmail.com" target="_blank">michael.blumenkrantz@gmail.com</a>> wrote:<br>
> > ><br>
> > > Hi,<br>
> > ><br>
> > > It's recently come to my attention that gitlab has Approvals. Was anyone else aware of this feature? You can just click a button and have your name recorded in the system as having signed off on landing a patch? Blew my mind.<br>
> > ><br>
> > > So with that being said, we also have this thing in the Mesa repo where everyone* has to constantly be adding these esoteric tags like Reviewed-by (I reviewed it), and Acked-by (I rubber stamped it), or Tested-by (I compiled it and maybe ran glxgears), and so forth.<br>
> > ><br>
> > > * Except some incredibly smart people already know where I'm going with this<br>
> > ><br>
> > > Instead of continuing to have to manually update each patch with the appropriate and definitely-unforgeable tags, what if we just used Approvals in the UI instead? We could then have marge-bot require approvals as needed in components and bring reviewing into the current year. Just think: no more rewriting all the commit logs and force-pushing the branch again before you merge!<br>
> > ><br>
> > > Anyway, I thought maybe this would be a nice idea to improve everyone's workflows. What do other people think?<br>
><br>
> My primary grip with approvals or the 👍 button is that it's the wrong<br>
> granularity.  It's per-MR instead of per-patch.  When people are<br>
> regularly posting MRs that touch a bunch of different stuff, per-patch<br>
> review is pretty common.  I'm not sure I want to lose that.  :-/<br>
<br>
Would it be an option to get Marge to not remove existing Rb tags, so<br>
we could get the streamlined process where possible and fall back if<br>
the MRs turn more complicated?<br></blockquote><div><br></div><div>If people really, truly care about per-patch Approval, couldn't they just split out patches from bigger MRs and get Approvals there? Otherwise it should be trivial enough to check the gitlab MR and see who reviewed which patch if it becomes an issue at a later date. Odds are at that point you're already going to the MR to see wtf someone was thinking...</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">
<br>
(as an aside I think we should just drop the tags in git, but I'll<br>
take anything that moves us forward)<br>
><br>
> --Jason<br>
><br>
> > I would love to see this be the process across Mesa.  We already don't<br>
> > rewrite commit messages for freedreno and i915g, and I only have to do<br>
> > the rebase (busy-)work for my projects in other areas of the tree.<br>
> ><br>
> > I don't think we should have marge-bot require approvals<br>
> > per-component, though.  There are times when an MR only incidentally<br>
> > touches a component (for example, changing function signatures in<br>
> > gallium), and actually getting a dev from every driver to sign off on<br>
> > it would be too much.<br>
</blockquote></div></div>