[Mesa-dev] [PATCH v2] bin/get-fixes-pick-list.sh: bring back the warning

[Andres Gomez]

On Fri, 2017-05-12 at 13:54 +0100, Emil Velikov wrote:
> On 12 May 2017 at 09:33, Andres Gomez <agomez at igalia.com> wrote:
> > We warn again if there is more than one line with the "fixes:" tag.
> > 
> > The warning is only silenced when the commit has landed already or we
> > output another message for every "fixes:" tag.
> > 
> 
> Since "only silenced" is no longer true, use something like the following?
> 
> "The warning is silenced when the commit has already landed or each
> fixes tag reference a commit that is in branch."
> 
> > Signed-off-by: Andres Gomez <agomez at igalia.com>
> > ---
> >  bin/get-fixes-pick-list.sh | 19 +++++++++++++------
> >  1 file changed, 13 insertions(+), 6 deletions(-)
> > 
> > diff --git a/bin/get-fixes-pick-list.sh b/bin/get-fixes-pick-list.sh
> > index cf95f28377..3ea649a0a4 100755
> > --- a/bin/get-fixes-pick-list.sh
> > +++ b/bin/get-fixes-pick-list.sh
> > @@ -33,7 +33,14 @@ do
> > 
> >         # For each one try to extract the tag
> >         fixes_count=`git show $sha | grep -i "fixes:" | wc -l`
> > +       warn=`(test $fixes_count -gt 1 && echo $fixes_count) || echo 0`
> >         while [ $fixes_count -gt 0 ] ; do
> > +               # Skip if it has been already landed.
> > +               if grep -q ^$sha already_picked ; then
> > +                       warn=0
> > +                       break
> > +               fi
> > +
> 
> Nit: please move this just after the cherry-ignore hunk.
> 
> >                 fixes=`git show $sha | grep -i "fixes:" | tail -n $fixes_count | head -n 1`
> 
> Are you sure we need the "tail -n $fixes_count | " here? Feel free to
> squash with this patch (+add small note in commit message) or address
> as follow-up.

It is needed, but your comment has made me realize that what it is
unnecessary is the " | head -n 1". I'll squash that in the patch and
add a note in the commit message before pushing.

> With the above
> Reviewed-by: Emil Velikov <emil.velikov at collabora.com>

Thanks for the review!

-- 
Br,

Andres