[Mesa-dev] [PATCH] bin/get-fixes-pick-list.sh: don't warn if more than one, go over them

[Andres Gomez]

Emil, to bring the warning back in the same conditions there is:
https://lists.freedesktop.org/archives/mesa-dev/2017-May/154907.html

Could you give it a look and may be grant a R-b ?

Thanks!

On Tue, 2017-05-09 at 10:42 +0100, Emil Velikov wrote:
> On 8 May 2017 at 14:49, Andres Gomez <agomez at igalia.com> wrote:
> > On Mon, 2017-05-08 at 13:41 +0100, Emil Velikov wrote:
> > > On 8 May 2017 at 11:52, Andres Gomez <agomez at igalia.com> wrote:
> > > > On Mon, 2017-05-08 at 10:56 +0100, Emil Velikov wrote:
> > > > > On 6 May 2017 at 15:09, Andres Gomez <agomez at igalia.com> wrote:
> > > > > > If an identified commit was having more than one fix, we would warn
> > > > > > about that and only treat the first.
> > > > > > 
> > > > > > Now, we don't warn but treat all of them.
> > > > > > 
> > > > > 
> > > > > Having read through this a couple of times, I'm not sure I fully parse it.
> > > > > 
> > > > > Are you saying that you want to flesh out all the commits when there's
> > > > > multiple Fixes tags?
> > > > 
> > > > I'm not sure I'm understanding this previous sentence ...
> > > > 
> > > 
> > > What is the exact intent of the patch?
> > > 
> > > > > AFIACT that is not a wise idea, since a handful are corner cases like:
> > > > >  - Fixes:\n $sha, Fixes: $piglit
> > > > >  - Fixes: $sha.*Fixes:
> > > > > and others.
> > > > 
> > > > I'm unsure I get which is the problem you see in those 2 examples. At
> > > > least, not if it is not a problem that was already happening with the
> > > > cases in which only one "Fixes" tag was found.
> > > > 
> > > > Maybe you can elaborate on that?
> > > > 
> > > 
> > > The current WARNING approach was deliberately introduced. If commit
> > > has multiple tags that means:
> > >  - It might need to be split - depending on how severe the issue is
> > >  - something has gone wrong - typo, miss-nomination, etc and would
> > > require a few seconds more than the ordinary nominations.
> > > 
> > > AFAICT your goal is to have the script produce a dummy list that one
> > > can copy/paste, but we do not want that here.
> > 
> > Not really.
> > 
> > The goal of the patch is not producing as output commits that only fix
> > commits that have not made it for the branch.
> > 
> 
> Right, thanks for the correction. This is exactly why I asked for
> clarification ;-)
> 
> 
> > The only cases in which I see this could cause a regression are these:
> >   - Fixes:\n $sha, Fixes: whatever
> >   - Fixes: whatever.*$sha, Fixes:
> > whatever
> > 
> > Which, in my eyes, are not any different to these already problematic
> > cases:
> >   - Fixes:\n $sha
> >   - Fixes: whatever.*$sha
> > 
> > Those need to be faced but I still don't see an easy way to solve them.
> > 
> 
> Indeed they are not that different. Note that the original script
> _will_ flag the former while the updated one will not.
> 
> > In other words, I will try to follow up with another patch(es) to
> > tackle those but my conclusion is that these scripts are only helpers
> > and the need to go over the commit list was needed before and is needed
> > after this patch.
> > 
> 
> Yes, they are - which is why I struggle to see why one will make them
> ignore potential uses patches :-\
> As said before - there is less than 5 instances flagged up every few
> weeks. And with the cherry-ignore additions (thanks for those) noise
> will be kept to minimum.
> 
> -Emil
> 
-- 
Br,

Andres