All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Elijah Newren via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Victoria Dye <vdye@github.com>,
	Derrick Stolee <stolee@gmail.com>,
	Lessley Dennington <lessleydennington@gmail.com>,
	Elijah Newren <newren@gmail.com>
Subject: Re: [PATCH v2 4/5] Update documentation related to sparsity and the skip-worktree bit
Date: Wed, 16 Feb 2022 10:15:25 +0100	[thread overview]
Message-ID: <220216.86k0dvupv7.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <0af00779128e594aff0ee4ec5378addeac8e88a2.1642175983.git.gitgitgadget@gmail.com>


On Fri, Jan 14 2022, Elijah Newren via GitGitGadget wrote:

> From: Elijah Newren <newren@gmail.com>
>
> Make several small updates, to address a few documentation issues
> I spotted:
>   * sparse-checkout focused on "patterns" even though the inputs (and
>     outputs in the case of `list`) are directories in cone-mode
>   * The description section of the sparse-checkout documentation
>     was a bit sparse (no pun intended), and focused more on internal
>     mechanics rather than end user usage.  This made sense in the
>     early days when the command was even more experimental, but let's
>     adjust a bit to try to make it more approachable to end users who
>     may want to consider using it.  Keep the scary backward
>     compatibility warning, though; we're still hard at work trying to
>     fix up commands to behave reasonably in sparse checkouts.
>   * both read-tree and update-index tried to describe how to use the
>     skip-worktree bit, but both predated the sparse-checkout command.
>     The sparse-checkout command is a far easier mechanism to use and
>     for users trying to reduce the size of their working tree, we
>     should recommend users to look at it instead.
>   * The update-index documentation pointed out that assume-unchanged
>     and skip-worktree sounded similar but had different purposes.
>     However, it made no attempt to explain the differences, only to
>     point out that they were different.  Explain the differences.
>   * The update-index documentation focused much more on (internal?)
>     implementation details than on end-user usage.  Try to explain
>     its purpose better for users of update-index, rather than
>     fellow developers trying to work with the SKIP_WORKTREE bit.
>   * Clarify that when core.sparseCheckout=true, we treat a file's
>     presence in the working tree as being an override to the
>     SKIP_WORKTREE bit (i.e. in sparse checkouts when the file is
>     present we ignore the SKIP_WORKTREE bit).
>
> Note that this commit, like many touching documentation, is best viewed
> with the `--color-words` option to diff/log.
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  Documentation/git-read-tree.txt       | 12 +++--
>  Documentation/git-sparse-checkout.txt | 76 ++++++++++++++++-----------
>  Documentation/git-update-index.txt    | 57 +++++++++++++++-----
>  3 files changed, 98 insertions(+), 47 deletions(-)
>
> diff --git a/Documentation/git-read-tree.txt b/Documentation/git-read-tree.txt
> index 8c3aceb8324..99bb387134d 100644
> --- a/Documentation/git-read-tree.txt
> +++ b/Documentation/git-read-tree.txt
> @@ -375,9 +375,14 @@ have finished your work-in-progress), attempt the merge again.
>  SPARSE CHECKOUT
>  ---------------
>  
> +Note: The `update-index` and `read-tree` primitives for supporting the
> +skip-worktree bit predated the introduction of
> +linkgit:git-sparse-checkout[1].  Users are encouraged to use
> +`sparse-checkout` in preference to these low-level primitives.

I was honestly a bit confused about whether we were really referring to
the git-update-index and git-read-tree commands here, or some
sparse-checkout (re-)usage of the same as "primitives", but reading it
again & the commit message we're just talking about the commands here.

So this really just wants to assure readers that they're advised to use
the shiny porcelain command instead of the plumbing.

I think we should refer to these as e.g. "linkgit:git-update-index[1]"
not "`update-index`" here, and call them e.g. "plumbing commands"
instead of "primitives" here, which will address that (the reader
wonders "what's a primitive?").

> -Initialize and modify the sparse-checkout configuration, which reduces
> -the checkout to a set of paths given by a list of patterns.
> +This command is used to create sparse checkouts, which means that it
> +changes the working tree from having all tracked files present, to only
> +have a subset of them.  It can also switch which subset of files are
> +present, or undo and go back to having all tracked files present in the
> +working copy.

In terms of prose I think it's preferred to keep matter-of-fact "Slices
and dices apples, making them easier to eat" instead of "This command
slices and dices apples, which means that it's easier to eat them".

I've forgotten what the linguisting term for that is, but it's more
consistent with our docs, and makes for easier reading.

> +The subset of files is chosen by providing a list of directories in
> +cone mode (which is recommended), or by providing a list of patterns
> +in non-cone mode.

As someone with light familiarity with sparse-checkout:

I'm still not sure if this is telling me that it's preferred to list
directories v.s. patterns, or if it's telling me it's better to operate
in "cone mode" v.s. "non-cone mode", or some combination of the two.

IOW I think peeling out that "(which is recommended)" and making it
clearly refer to which (or both?) of the two we're talking about would
be much better.

> +When in a sparse-checkout, other Git commands behave a bit differently.
> +For example, switching branches will not update paths outside the
> +sparse-checkout directories/patterns, and `git commit -a` will not record
> +paths outside the sparse-checkout directories/patterns as deleted.

(I didn't read through the rest in any detail)

  reply	other threads:[~2022-02-16  9:24 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-13 16:43 [PATCH 0/5] Remove the present-despite-SKIP_WORKTREE class of bugs (for sparse-checkouts) Elijah Newren via GitGitGadget
2022-01-13 16:43 ` [PATCH 1/5] t1011: add testcase demonstrating accidental loss of user modifications Elijah Newren via GitGitGadget
2022-01-13 16:43 ` [PATCH 2/5] unpack-trees: fix accidental loss of user changes Elijah Newren via GitGitGadget
2022-01-13 16:43 ` [PATCH 3/5] repo_read_index: clear SKIP_WORKTREE bit from files present in worktree Elijah Newren via GitGitGadget
2022-01-13 16:43 ` [PATCH 4/5] Update documentation related to sparsity and the skip-worktree bit Elijah Newren via GitGitGadget
2022-01-13 16:43 ` [PATCH 5/5] Accelerate clear_skip_worktree_from_present_files() by caching Elijah Newren via GitGitGadget
2022-01-13 23:35   ` Elijah Newren
2022-01-14 15:59 ` [PATCH v2 0/5] Remove the present-despite-SKIP_WORKTREE class of bugs (for sparse-checkouts) Elijah Newren via GitGitGadget
2022-01-14 15:59   ` [PATCH v2 1/5] t1011: add testcase demonstrating accidental loss of user modifications Elijah Newren via GitGitGadget
2022-02-16  8:51     ` Ævar Arnfjörð Bjarmason
2022-02-16 16:02       ` Elijah Newren
2022-01-14 15:59   ` [PATCH v2 2/5] unpack-trees: fix accidental loss of user changes Elijah Newren via GitGitGadget
2022-01-14 15:59   ` [PATCH v2 3/5] repo_read_index: clear SKIP_WORKTREE bit from files present in worktree Elijah Newren via GitGitGadget
2022-02-16  8:57     ` Ævar Arnfjörð Bjarmason
2022-02-16 16:08       ` Elijah Newren
2022-02-19  1:06     ` Jonathan Nieder
2022-02-19 16:42       ` Elijah Newren
2022-02-19 18:14         ` Jonathan Nieder
2022-02-20  5:28           ` Elijah Newren
2022-02-20 16:56       ` Derrick Stolee
2022-02-22 23:17         ` Jonathan Nieder
2022-01-14 15:59   ` [PATCH v2 4/5] Update documentation related to sparsity and the skip-worktree bit Elijah Newren via GitGitGadget
2022-02-16  9:15     ` Ævar Arnfjörð Bjarmason [this message]
2022-02-16 16:21       ` Elijah Newren
2022-01-14 15:59   ` [PATCH v2 5/5] Accelerate clear_skip_worktree_from_present_files() by caching Elijah Newren via GitGitGadget
2022-01-15  1:39     ` Victoria Dye
2022-02-16  9:32     ` Ævar Arnfjörð Bjarmason
2022-02-16 16:30       ` Elijah Newren
2022-02-17  4:40         ` Elijah Newren
2022-01-15  1:51   ` [PATCH v2 0/5] Remove the present-despite-SKIP_WORKTREE class of bugs (for sparse-checkouts) Victoria Dye

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=220216.86k0dvupv7.gmgdl@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=lessleydennington@gmail.com \
    --cc=newren@gmail.com \
    --cc=stolee@gmail.com \
    --cc=vdye@github.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.