On Thu, Sep 16, 2021 at 09:40:54AM +0300, Amir Goldstein wrote:

On Thu, Sep 16, 2021 at 2:43 AM Darrick J. Wong [off-list ref] wrote:

quoted

From: Darrick J. Wong <djwong@kernel.org>

The 'other' group is vaguely defined at best -- other than what?  It's
not clear what tests belong in this group, and it has become a dumping
ground for random stuff that are classified in other groups.  Don't let
people create new other group tests.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 new |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/new b/new
index 6b7dc5d4..5cf96c50 100755
--- a/new
+++ b/new

@@ -96,9 +96,9 @@ then

     while true
     do
-       echo -n "Add to group(s) [other] (separate by space, ? for list): "
+       echo -n "Add to group(s) [auto] (separate by space, ? for list): "
        read ans
-       [ -z "$ans" ] && ans=other
+       [ -z "$ans" ] && ans=auto
        if [ "X$ans" = "X?" ]
        then
            echo $(group_names)

@@ -109,6 +109,9 @@ then
                echo "Invalid characters in group(s): $inval"
                echo "Only lower cases, digits and underscore are allowed in groups, separated by space"
                continue
+           elif echo "$ans" | grep -q -w "other"; then
+               echo "Do not add more tests to group \"other\"."
+               continue

Should we also filter out "other" from group_names(), so it is not listed
for "?"?

No; there are drawbacks to that, as you point out below.

With this patch, "other" does not emit a warning when passed in as a script
command line argument.

Done.

If we filter "other" from group_names(), then the warning in "expert mode"
will be a bit confusing (group "other" not defined in documentation).

I will filter it out in the specific case case that the interactive user
specified "?" to list the groups.

Also, it is not clear to me if this is intentional behavior that interactive
mode allows non-dcumented groups (with valid chars validation) and
expert mode does not allow non-documented groups?

Probably not.

It may be simpler to use the same helper in both modes (is_group_valid)
to emit the correct warning and either proceed (expert mode) or get
back to prompt (interactive mode).

This is getting farther afield from where I wanted this thing to go.
Very well, I'll split the ./new cleanups into its own series, but TBH
I've gotten tired of people asking for more and more cleanups out of me.

--D

Thanks,
Amir.