[Mesa-dev] [PATCH 2/2] bin/get-fixes-pick-list.sh: better identify multiple "fixes:" tags

[Emil Velikov]

On 15 June 2017 at 13:55, Andres Gomez <agomez at igalia.com> wrote:
> On Mon, 2017-05-15 at 11:56 +0100, Emil Velikov wrote:
>> On 13 May 2017 at 01:11, Andres Gomez <agomez at igalia.com> wrote:
>> > We were not considering as multiple fixes lines with:
>> > Fixes: $sha_1, Fixes: $sha_2
>> >
>> > Now, we split the lines so we will consider them individually, as in:
>> > Fixes: $sha_1,
>> > Fixes: $sha_2
>> >
>> > Additionally, we try to get the SHA from split lines so:
>> > Fixes:
>> > $sha_1
>> >
>> > Will be considered as:
>> > Fixes: $sha_1
>> >
>> > Signed-off-by: Andres Gomez <agomez at igalia.com>
>> > ---
>> >  bin/get-fixes-pick-list.sh | 12 ++++++++----
>> >  1 file changed, 8 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/bin/get-fixes-pick-list.sh b/bin/get-fixes-pick-list.sh
>> > index f9afcc49ce..32d830cda0 100755
>> > --- a/bin/get-fixes-pick-list.sh
>> > +++ b/bin/get-fixes-pick-list.sh
>> > @@ -36,14 +36,18 @@ do
>> >                 continue
>> >         fi
>> >
>> > +       # Place every "fixes:" tag on its own line and join with the next word
>> > +       # on its line or a later one.
>> > +       fixes=`git show -s $sha | tr -d "\n" | sed -e 's/fixes:/\nfixes:/Ig' | grep "fixes:" | sed -e 's/\(fixes:\)[[:space:]]*\([a-zA-Z0-9]*\).*$/\1\2/g'`
>>
>> Do we need the tr -d?
>
> If what we are intending is that each new line will start with the
> "fixes:" tag then, yes, I think it is needed.
>
> I tried to see how to do this with sed and found it much more
> cumbersome and felt using "tr -d" much easier to do and understand.
>
Agreed.

>> Nit: Let's also handle any trailing whitespace after the tag.
>> Something like the following should do it.
>>
>> 's/fixes:[[:space:]]*/\nfixes:/Ig'
>
> This is already done by the second sed. I'll move it to the first one
> but I don't think we are really gaining anything ...
>
Agreed.

> I'll push this by now ...
>
Ack. Thanks for polishing the script.

-Emil