public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox•com>
To: Todd Zullinger <tmz@pobox•com>,
	Jonathan Tan <jonathantanmy@google•com>,
	Derrick Stolee <stolee@gmail•com>
Cc: git@vger•kernel.org, Johannes Schindelin <johannes.schindelin@gmx•de>
Subject: Re: [PATCH] doc: builtin add -i is enabled by feature.experimental
Date: Wed, 16 Jun 2021 11:20:45 +0900	[thread overview]
Message-ID: <xmqq4kdyturm.fsf@gitster.g> (raw)
In-Reply-To: <20210615164522.1079951-1-tmz@pobox.com> (Todd Zullinger's message of "Tue, 15 Jun 2021 12:45:22 -0400")

Todd Zullinger <tmz@pobox•com> writes:

> Note that add.interactive.useBuiltin is enabled by feature.experimental.
> It was added in 2df2d81ddd (add -i: use the built-in version when
> feature.experimental is set, 2020-09-08).
>
> Signed-off-by: Todd Zullinger <tmz@pobox•com>

> ---
> I was checking my configuration to see if I still needed to have
> add.interactive.useBuiltin set and noticed that it wasn't listed in the
> settings enabled by feature.experimental.

So together with the fetch.negotiationAlgorithm only two knobs are
affected by feature.experimental?  Or is this patch only about add-i
because that is the only thing you found out about?

Explicitly state that these are the only two that are affected by
this bit in the log message so that readers of "git log" do not have
to ask the question.

The other configuration feature.experimental controls is described
in Documentation/config/fetch.txt like this:

    fetch.negotiationAlgorithm::
            Control how information about the commits in the local repository is
            sent when negotiating the contents of the packfile to be sent by the
            ...
            The default is "default" which instructs Git to use the default algorithm
            that never skips commits (unless the server has acknowledged it or one
            of its descendants). If `feature.experimental` is enabled, then this
            setting defaults to "skipping".
            Unknown values will cause 'git fetch' to error out.

> diff --git a/Documentation/config/add.txt b/Documentation/config/add.txt
> index c9f748f81c..7d6d325571 100644
> --- a/Documentation/config/add.txt
> +++ b/Documentation/config/add.txt
> @@ -9,4 +9,5 @@ add.ignore-errors (deprecated)::
>  add.interactive.useBuiltin::
>  	[EXPERIMENTAL] Set to `true` to use the experimental built-in
>  	implementation of the interactive version of linkgit:git-add[1]
> -	instead of the Perl script version. Is `false` by default.
> +	instead of the Perl script version.  If `feature.experimental` is
> +	enabled, this setting is `true`.  By default, it is `false`.

The added sentence doesn't exactly make sense.  If the experimental
bit is set, this setting _defaults_ to true.  If you set this
explicitly, the experimental bit would not override it, would it?

I prefer to see things described only once, so I am wondering if we
can get away by doing these instead:

 - Start the description of fetch.negotiationAlgorithm with
   [EXPERIMENTAL] like this one does, and remove the sentence about
   the experimental bit from there.

 - Leave the description of add.interactive.useBuiltin as is.

 - Use the change you made to Documentation/config/feature.txt below.

Additionally, it may help readers without hurting maintainability to
say "[EXPERIMENTAL] (see also `feature.experimental`)" to refer them
to the description of the defaults set by the experimental bit.

Alternatively, we can 

 - Remove the description of other configuration variables affected
   by feature.experimental from the description of the experimental
   bit in Documentation/config/feature.txt, and replace it with "The
   default values for configuration variables marked with
   '[EXPERIMENTAL]' are affected by setting this bit", and

 - Start the description of fetch.negotiationAlgorithm with
   [EXPERIMENTAL] like this one does, and remove the sentence about
   the experimental bit from there.

 - Use the change you made to Documentation/config/add.txt as is.

That would also reduce duplicates.  From organizational point of
view, I prefer the latter slightly more.  It makes the readers who
want to learn what knobs would be tweaked to run "grep" on the
documentation set, but it may not be too much to expect from the
more adventuous types.  I dunno.

I am sending this also to Derrick (for adding the experimental bit
and threw negotiate knob into it) and Jonathan Tan (for adding and
describing negotiate) for their input.

Thanks.

> diff --git a/Documentation/config/feature.txt b/Documentation/config/feature.txt
> index cdecd04e5b..caaa97dc61 100644
> --- a/Documentation/config/feature.txt
> +++ b/Documentation/config/feature.txt
> @@ -12,6 +12,10 @@ feature.experimental::
>  	setting if you are interested in providing feedback on experimental
>  	features. The new default values are:
>  +
> +* `add.interactive.useBuiltin=true` which enables the built-in implementation
> +of the interactive version of linkgit:git-add[1] instead of the Perl script
> +version.
> ++
>  * `fetch.negotiationAlgorithm=skipping` may improve fetch negotiation times by
>  skipping more commits at a time, reducing the number of round trips.
>  
>  feature.manyFiles::
>  	Enable config options that optimize for repos with many files in the

  parent reply	other threads:[~2021-06-16  2:20 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-15 16:45 [PATCH] doc: builtin add -i is enabled by feature.experimental Todd Zullinger
2021-06-15 19:24 ` Ævar Arnfjörð Bjarmason
2021-06-16  2:20 ` Junio C Hamano [this message]
2021-06-16 17:38   ` Derrick Stolee
2021-06-17 12:01 ` Johannes Schindelin

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=xmqq4kdyturm.fsf@gitster.g \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=johannes.schindelin@gmx$(echo .)de \
    --cc=jonathantanmy@google$(echo .)com \
    --cc=stolee@gmail$(echo .)com \
    --cc=tmz@pobox$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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