[PATCH] xfstests comma separated group names

[PATCH] xfstests comma separated group names

[Itaru Kitayama]

[-- Attachment #1.1: Type: text/plain, Size: 1507 bytes --]

This patch lets the -g option take multiple group names.

Signed-off-By: Itaru Kitayama <itaru.kitayama@gmail.com>

 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
+               for g in ${group//,/ }; do
+                       group_list=$(get_group_list $g)
+                       if [ -z "$group_list" ]; then
+                       echo "Group \"$g\" 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
+                       done
                done
-
                ;;

        -x)     xgroup=$2 ; shift ;

[-- Attachment #1.2: Type: text/html, Size: 2201 bytes --]

[-- Attachment #2: Type: text/plain, Size: 121 bytes --]

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfstests comma separated group names

[Dave Chinner]

On Mon, May 20, 2013 at 05:29:29PM +0900, Itaru Kitayama wrote:
> This patch lets the -g option take multiple group names.
> 
> Signed-off-By: Itaru Kitayama <itaru.kitayama@gmail.com>

You can already run multiple test groups by specifying multiple -g
options. e.g:

# check -g rw -g attr

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfstests comma separated group names

[Itaru Kitayama]

[-- Attachment #1.1: Type: text/plain, Size: 559 bytes --]

Yes, we can do that, but I just thought as you originally proposed last year

-g rw,attr reads better.

Itaru


On Mon, May 20, 2013 at 8:30 PM, Dave Chinner <david@fromorbit.com> wrote:

> On Mon, May 20, 2013 at 05:29:29PM +0900, Itaru Kitayama wrote:
> > This patch lets the -g option take multiple group names.
> >
> > Signed-off-By: Itaru Kitayama <itaru.kitayama@gmail.com>
>
> You can already run multiple test groups by specifying multiple -g
> options. e.g:
>
> # check -g rw -g attr
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com
>

[-- Attachment #1.2: Type: text/html, Size: 1163 bytes --]

[-- Attachment #2: Type: text/plain, Size: 121 bytes --]

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfstests comma separated group names

[Dave Chinner]

On Mon, May 20, 2013 at 08:42:35PM +0900, Itaru Kitayama wrote:
> Yes, we can do that, but I just thought as you originally proposed last year
> 
> -g rw,attr reads better.

Agreed, it does read better and is easily to type.

That's what you need to put in the commit message - why it is better
than what we currently have, and whether it is badwards compatible
or not (doesn't break any existing scripts). i.e. the commit
message is for telling people -why- the change should be made as we
can look at the code to determine -what- the change is.  ;)

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfstests comma separated group names

[Itaru Kitayama]

[-- Attachment #1.1: Type: text/plain, Size: 2442 bytes --]

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@redhat.com>
Signed-off-by: Itaru Kitayama <itaru.kitayama@gmail.com>

---

 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
+               for g in ${group//,/ }; do
+                       group_list=$(get_group_list $g)
+                       if [ -z "$group_list" ]; then
+                       echo "Group \"$g\" 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
+                       done
                done
-
                ;;

        -x)     xgroup=$2 ; shift ;


On Tue, May 21, 2013 at 9:45 AM, Dave Chinner <david@fromorbit.com> wrote:

> On Mon, May 20, 2013 at 08:42:35PM +0900, Itaru Kitayama wrote:
> > Yes, we can do that, but I just thought as you originally proposed last
> year
> >
> > -g rw,attr reads better.
>
> Agreed, it does read better and is easily to type.
>
> That's what you need to put in the commit message - why it is better
> than what we currently have, and whether it is badwards compatible
> or not (doesn't break any existing scripts). i.e. the commit
> message is for telling people -why- the change should be made as we
> can look at the code to determine -what- the change is.  ;)
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com
>

[-- Attachment #1.2: Type: text/html, Size: 6163 bytes --]

[-- Attachment #2: Type: text/plain, Size: 121 bytes --]

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfstests comma separated group names

[Dave Chinner]

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@redhat.com>

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@gmail.com>
> 
> ---
> 
>  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.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfstests comma separated group names

[Eric Sandeen]

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@redhat.com>
> 
> 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@gmail.com>
>>
>> ---
>>
>>  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.
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs