From mboxrd@z Thu Jan 1 00:00:00 1970 From: Radim =?utf-8?B?S3LEjW3DocWZ?= Date: Tue, 16 Aug 2016 16:03:50 +0000 Subject: Re: [kvm-unit-tests PATCH V3 1/5] scripts/runtime: Add ability to mark test as don't run by default Message-Id: <20160816160349.GD12385@potion> List-Id: References: <1471331895-29887-1-git-send-email-sjitindarsingh@gmail.com> <20160816120037.32l5sezhhsgtldxa@kamzik.localdomain> In-Reply-To: <20160816120037.32l5sezhhsgtldxa@kamzik.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Andrew Jones Cc: Suraj Jitindar Singh , kvm@vger.kernel.org, pbonzini@redhat.com, kvm-ppc@vger.kernel.org, lvivier@redhat.com, thuth@redhat.com 2016-08-16 14:00+0200, Andrew Jones: > On Tue, Aug 16, 2016 at 05:18:11PM +1000, Suraj Jitindar Singh wrote: >> diff --git a/scripts/runtime.bash b/scripts/runtime.bash >> @@ -32,6 +32,26 @@ get_cmdline() >> +skip_nodefault() >> +{ >> + while true; do >> + read -p "Test marked not to be run by default, are you sure (Y/N)? " yn I'd write "run this test" instead of "are you sure", or something similar for the question. The user can be sure that the has is marked with nodefault. ;) >> + case $yn in >> + "Y" | "y" | "Yes" | "yes") > > What about "YES" :-) And exclamation marks! "YES!!!" > Actually, I'd just accept 'Y' for yes, and nothing else, like the prompt says. NO. If it is only one value, then make it "y". motto: saving the Earth, one shift at a time. This kind of user interface usually accepts at least "[yY]|[yY]es" ... users will already be pissed that they have to input something and denying a perfectly logical "yes" (which is what "y" stands for) is going too overboard, IMO. > And, instead of looping for valid input, all other input can just mean no. "y/N" is the convention for writing a bool question that defaults to no. I'd accept "" (just enter) as the default and then, looping isn't unexpected and user already typed some crap in that case, so they probably want to answer the question without having to run the command again. >> + return 1 >> + ;; >> + "N" | "n" | "No" | "no" | "q" | "quit" | "exit") > > We should output something when the answer is 'no' like "User aborted", > or whatever. > >> + exit Wouldn't "return 1" and the SKIP message be enough? >> + ;; >> + *) >> + echo Please select Y or N Just asking the question again is more common -- it's not hard to figure out that the answer was not accepted. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Radim =?utf-8?B?S3LEjW3DocWZ?= Subject: Re: [kvm-unit-tests PATCH V3 1/5] scripts/runtime: Add ability to mark test as don't run by default Date: Tue, 16 Aug 2016 18:03:50 +0200 Message-ID: <20160816160349.GD12385@potion> References: <1471331895-29887-1-git-send-email-sjitindarsingh@gmail.com> <20160816120037.32l5sezhhsgtldxa@kamzik.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Suraj Jitindar Singh , kvm@vger.kernel.org, pbonzini@redhat.com, kvm-ppc@vger.kernel.org, lvivier@redhat.com, thuth@redhat.com To: Andrew Jones Return-path: Received: from mx1.redhat.com ([209.132.183.28]:58350 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753259AbcHPQDy (ORCPT ); Tue, 16 Aug 2016 12:03:54 -0400 Content-Disposition: inline In-Reply-To: <20160816120037.32l5sezhhsgtldxa@kamzik.localdomain> Sender: kvm-owner@vger.kernel.org List-ID: 2016-08-16 14:00+0200, Andrew Jones: > On Tue, Aug 16, 2016 at 05:18:11PM +1000, Suraj Jitindar Singh wrote: >> diff --git a/scripts/runtime.bash b/scripts/runtime.bash >> @@ -32,6 +32,26 @@ get_cmdline() >> +skip_nodefault() >> +{ >> + while true; do >> + read -p "Test marked not to be run by default, are you sure (Y/N)? " yn I'd write "run this test" instead of "are you sure", or something similar for the question. The user can be sure that the has is marked with nodefault. ;) >> + case $yn in >> + "Y" | "y" | "Yes" | "yes") > > What about "YES" :-) And exclamation marks! "YES!!!" > Actually, I'd just accept 'Y' for yes, and nothing else, like the prompt says. NO. If it is only one value, then make it "y". motto: saving the Earth, one shift at a time. This kind of user interface usually accepts at least "[yY]|[yY]es" ... users will already be pissed that they have to input something and denying a perfectly logical "yes" (which is what "y" stands for) is going too overboard, IMO. > And, instead of looping for valid input, all other input can just mean no. "y/N" is the convention for writing a bool question that defaults to no. I'd accept "" (just enter) as the default and then, looping isn't unexpected and user already typed some crap in that case, so they probably want to answer the question without having to run the command again. >> + return 1 >> + ;; >> + "N" | "n" | "No" | "no" | "q" | "quit" | "exit") > > We should output something when the answer is 'no' like "User aborted", > or whatever. > >> + exit Wouldn't "return 1" and the SKIP message be enough? >> + ;; >> + *) >> + echo Please select Y or N Just asking the question again is more common -- it's not hard to figure out that the answer was not accepted.