git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, "SZEDER Gábor" <szeder.dev@gmail.com>,
	"Johannes Schindelin" <johannes.schindelin@gmx.de>
Subject: Re: [PATCH 2/2] tests: introduce --stress-jobs=<N>
Date: Sun, 03 Mar 2019 18:55:05 +0900	[thread overview]
Message-ID: <xmqqh8ck1dvq.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: 281d3f1d19d1c93e8d1e66ae16fe3fb286554c0a.1551561582.git.gitgitgadget@gmail.com

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> The --stress option currently accepts an argument, but it is confusing
> to at least this user that the argument does not define the maximal
> number of stress iterations, but instead the number of jobs to run in
> parallel per stress iteration.

Yeah, when there are multiple knobs that can take integral value,
and especially when the knobs are of equal usefulness, the users
would happen pick the right one 50% of the time by accident, which
is not a happy state.

> Let's introduce a separate option for that, whose name makes it more
> obvious what it is about, and let --stress=<N> error out with a helpful
> suggestion about the two options tha could possibly have been meant.

Good.  

Making --stress=<value> error out (instead of deprecating with a
transition plan) is probably OK given its audience.  It is a good
trade-off to save our braincycles by not having to worry about
transition and forcing everybody to adjust (I am assuming that
nobody has a wrapper script to run tXXXX-title.sh scripts that
passes the --stress=<value> thing).

t/README must be updated as --stress=<N> is documented to specify
the degree of parallelism, though.

I'll queue in the meantime, to reduce the risk of forgetting the
topic.

Thanks.

> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  t/test-lib.sh | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index ab7f27ec6a..6e557982a2 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -142,10 +142,16 @@ do
>  	--stress)
>  		stress=t ;;
>  	--stress=*)
> +		echo "error: --stress does not accept an argument: '$opt'" >&2
> +		echo "did you mean --stress-jobs=${opt#*=} or --stress-limit=${opt#*=}?" >&2
> +		exit 1
> +		;;
> +	--stress-jobs=*)
> +		stress=t;
>  		stress=${opt#--*=}
>  		case "$stress" in
>  		*[!0-9]*|0*|"")
> -			echo "error: --stress=<N> requires the number of jobs to run" >&2
> +			echo "error: --stress-jobs=<N> requires the number of jobs to run" >&2
>  			exit 1
>  			;;
>  		*)	# Good.

  parent reply	other threads:[~2019-03-03  9:55 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-02 21:19 [PATCH 0/2] tests: some touchups related to the --stress feature Johannes Schindelin via GitGitGadget
2019-03-02 21:19 ` [PATCH 1/2] tests: let --stress-limit=<N> imply --stress Johannes Schindelin via GitGitGadget
2019-03-03  9:54   ` Junio C Hamano
2019-03-02 21:19 ` [PATCH 2/2] tests: introduce --stress-jobs=<N> Johannes Schindelin via GitGitGadget
2019-03-03  2:30   ` Eric Sunshine
2019-03-03  9:55   ` Junio C Hamano [this message]
2019-03-03 14:19   ` SZEDER Gábor
2019-03-03 14:47     ` Johannes Schindelin
2019-03-03 14:44 ` [PATCH v2 0/2] tests: some touchups related to the --stress feature Johannes Schindelin via GitGitGadget
2019-03-03 14:44   ` [PATCH v2 1/2] tests: let --stress-limit=<N> imply --stress Johannes Schindelin via GitGitGadget
2019-03-03 14:44   ` [PATCH v2 2/2] tests: introduce --stress-jobs=<N> Johannes Schindelin via GitGitGadget
2019-03-03 15:00     ` Eric Sunshine
2019-03-04  3:22       ` Junio C Hamano
2019-03-04  3:55         ` Eric Sunshine
2019-03-03 17:45     ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqh8ck1dvq.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=szeder.dev@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).