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

[Emil Velikov]

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