Thread (121 messages) flat view 121 messages, 8 authors, 2022-02-16

Re: [PATCH v3 6/6] worktree: copy sparse-checkout patterns and config on add

From: Eric Sunshine <hidden>
Date: 2021-12-30 08:16:14

On Wed, Dec 29, 2021 at 5:45 PM Elijah Newren [off-list ref] wrote:
On Wed, Dec 29, 2021 at 1:39 PM Derrick Stolee [off-list ref] wrote:
quoted
I think the motivation is that worktree config is something that is
harder to set up than to just run a 'git config' command, and we
should guide users into a best practice for using it. The
documentation becomes "run this command to enable it".
Okay, but that's an answer to a different question -- namely, "if
users want/need to explicitly set it up, why should we have a
command?"  Your answer here is a very good answer to that question,
but you've assumed the "if".  My question was on the "if": (Why) Do
users need or want to explicitly set it up?

Secondarily, if users want to set it up explicitly, is the work here
really sufficient to help guide them?  In particular, I discovered and
started using extensions.worktreeConfig without ever looking at the
relevant portions of git-worktree.txt (the references in
git-config.txt never mentioned them).  I also pushed this usage to
others, including even to you with `git-sparse-checkout`, and no
reviewer on this list ever caught it or informed me of the `proper`
additional guidelines found in git-worktree.txt until this thread
started.  So, relying on folks to read git-worktree.txt for this
config item feels a bit weak to me.  Granted, your new command will be
much more likely to be read since it appears near the top of
git-worktree.txt, but I just don't think that's enough.  The
references to extensions.worktreeConfig in git-config.txt should
reference any special command or extended steps if we expect users to
manually configure it (whether via explicit new subcommand or via also
playing with other config settings).
Agreed about it being a good idea to update git-config.txt to mention
the extra bookkeeping related to `extensions.worktreeConfig=1` (though
it doesn't necessarily need to be done by this series).
Anyway, if we think users want to set it up explicitly, and we address
the discoverability problem above, then I'd vote for
"independent-config" or "private-config" (or _maybe_
"migrate-config").  Because:
  * no sense repeating the word `worktree` in `git worktree
init-worktree-config`.  It's redundant.
  * The words "independent" or "private" suggest what it does and why
users might want to use the new subcommand.
  * It's not an "init":
I had some similar thoughts/objections to the name
`init-worktree-config` (not that I was able to come up with anything
better). Thanks for enumerating them here. Anyhow, as you say below,
the new subcommand isn't really needed.
    * The documentation makes no attempt to impose a temporal order of
using this command before `git worktree add`.  (Would we even want
to?)
Aside from possible(?) sparse-checkout issues, I don't think there is
a reason to impose temporal order in general.
    * As per my recommendation elsewhere, this step just isn't needed
for the vast majority of users (i.e. those with non-bare clones who
leave core.worktree alone).
    * ...and it's also not needed for other (core.bare=true or
core.worktree set) users since `git worktree add` will automatically
run this config migration for them
Your enumeration at the very end of [1] pretty well convinced me that
we don't need this command; certainly not at present, and perhaps
never.
And, actually, with the name "independent-config" or "private-config",
I might be answering my own question.  It's a name that speaks to why
users might want it, so my objection to the new command is rapidly
diminishing.
Perhaps some day it might make sense to add such a subcommand (more
suitably named), but with your proposal in [1] it's hard to justify
adding it now, and it certainly doesn't need to be part of this patch
series.

[1]: https://lore.kernel.org/git/CABPp-BHuO3B366uJuODMQo-y449p8cAMVn0g2MTcO5di3Xa7Zg@mail.gmail.com/ (local)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help