xfs
[Top] [All Lists]

Re: [PATCH] xfstests comma separated group names

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH] xfstests comma separated group names
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Wed, 22 May 2013 09:12:56 -0500
Cc: Itaru Kitayama <itaru.kitayama@xxxxxxxxx>, xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20130521052118.GM24543@dastard>
References: <CANW9uytRpGXcd9x=HWMEoFvd3_9G98CN_Tv3YacTtd1h+Gq=4g@xxxxxxxxxxxxxx> <20130520113045.GB24543@dastard> <CANW9uytQDPaBr_VAGMuwSqhStCfyMCGmV_A3ccNzOExi-KeZfQ@xxxxxxxxxxxxxx> <20130521004553.GL24543@dastard> <CANW9uyvUgS6i+z0iUtyuJDe2YhbtnVYmYs87xB74=fJCMuG06Q@xxxxxxxxxxxxxx> <20130521052118.GM24543@dastard>
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:17.0) Gecko/20130509 Thunderbird/17.0.6
On 5/21/13 12:21 AM, Dave Chinner wrote:
> On Tue, May 21, 2013 at 12:45:32PM +0900, Itaru Kitayama wrote:
>> In the current check script the -g option assumes only one group is given.
>> With this patch, the -g option
>> understands comma separated multiple groups as the argument as well.
>> Existing scripts are not affected
>> by this patch.
>>
>> Reviewed-by: Dave Chinner <david@xxxxxxxxxx>
> 
> No, I didn't. A comment on a patch is not a review.
> 
> The only time you should add tags like this is if you receive them
> in email from the person in question. Once I've actaully done a
> review of the code, I'll respond with such a tag. See
> Documentation/SubmittingPatches for more information about what the
> reviewed-by tag actually means...
> 
>> Signed-off-by: Itaru Kitayama <itaru.kitayama@xxxxxxxxx>
>>
>> ---
>>
>>  check |   23 ++++++++++++-----------
>>  1 file changed, 12 insertions(+), 11 deletions(-)
>>
>> diff --git a/check b/check
>> index a79747e..0e0f208 100755
>> --- a/check
>> +++ b/check
>> @@ -164,18 +164,19 @@ while [ $# -gt 0 ]; do
>>         -nfs)   FSTYP=nfs ;;
>>
>>         -g)     group=$2 ; shift ;
>> -               group_list=$(get_group_list $group)
>> -               if [ -z "$group_list" ]; then
>> -                   echo "Group \"$group\" is empty or not defined?"
>> -                   exit 1
>> -               fi
>> -
>> -               [ ! -s $tmp.list ] && touch $tmp.list
>> -               for t in $group_list; do
>> -                       grep -s "^$t\$" $tmp.list >/dev/null || \
>> -                                                       echo "$t"
>>>> $tmp.list
> 
> There's a whitespace and wrapping problem with your mailer. it's
> converting all tabs to spaces, and it's wrapping long lines in the
> patch. Please see Documentation/email-clients.txt for help to set
> your mailer up properly.

Dave is a tough teacher but you can learn a lot from him.  ;)

Thanks for sending the patch, I agree that this will be nice to have,
once the patch submission issues get fixed up.

-Eric

> Cheers,
> 
> Dave.
> 

<Prev in Thread] Current Thread [Next in Thread>