On 2/12/2022 7:39 PM, Elijah Newren via GitGitGadget wrote: > Note (reason for RFC): this is RFC primarily because of dependencies (you > may not want to pick this up yet, Junio), though there is also a question of > whether to split patch 7 into two steps -- one for now and one we take in > some future release. In particular, the first step could be to have > sparse-checkout error out if neither --no-cone nor --cone are specified and > then change the default to be --cone in some future release. I don't think > splitting it into two steps is needed given (a) users who are unaware of the > change will still get useful error messages telling them that directories > are expected due to patches 4-6 of this series, and (b) the huge > "EXPERIMENTAL" warning and explicit note about likely behavioral changes in > git-sparse-checkout.txt serves as warning about the changes. However, the > two step approach is an alternative. I support this change. This will also require an update to the 'git clone' documentation around the '--sparse' option, as I imagine we are going to be changing behavior there. (If not, then we should do that as part of the deprecation.) > Note 2 (dependencies): this depends on en/present-despite-skipped (which > depends on vd/sparse-clean-etc) and on > ds/sparse-checkout-requires-per-worktree-config, because of otherwise heavy > text conflicts in patch 7 to git-sparse-checkout.txt. Given that neither of > those have merged to next yet, it may be premature to pick up this series. Yes, hopefully things will start to settle down a little, especially since vd/sparse-clean-etc is due to merge any day now. > This series continues attempts to make sparse-checkouts more user friendly. > A quick overview: > > * Patches 1-2 fix existing bugs from en/sparse-checkout-set > * Patch 3 fixes sparse-checkout-from-subdirectories-ignores-"prefix" (see > https://lore.kernel.org/git/29f0410e-6dfa-2e86-394d-b1fb735e7608@xxxxxxxxx/), > at least in cone mode. In non-cone mode it is not clear if patch 3 is a > "fix" or a "break" (see the "NON-CONE PROBLEMS" section of the manual > added in patch 7, and > https://lore.kernel.org/git/e1934710-e228-adc4-d37c-f706883bd27c@xxxxxxxxx/ > where Stolee suggested it might be incorrect). > * Patches 4-6 check positional arguments to set/add and provide > errors/warnings for very likely mistakes. It also adds a --skip-checks > flag for overridding in case you have a very unusual situation. I took a close look at these patches and mostly have minor typo fixes. There was one behavior issue: I don't think you should warn for file paths in non- cone-mode. Being able to select a single file in a directory full of large files is one of the main reasons to use non-cone-mode, in my experience. It might be worth adding some documentation about how to reorganize a repo to fit cone mode patterns, but that's not necessary. > * Patch 7 makes cone mode the default, and makes large updates to the > documentation both to explain why we changed the default, and to simplify > the documentation since users can just use directories and ignore the > intricacies of gitignore-style patterns and how they relate to sparse > checkouts. I'm a fan of the end-result of this patch. I responded with some specific comments and a suggestion for splitting it into a series of four patches. Your first 6 patches are likely to be noncontroversial and could merge more quickly than the deprecation. I think it would be good to get the full deprecation under full review as soon as possible so we can give the community a long window to comment on it. We can also consider if we need a release or two where this behavior change is announced, but not actually done. I'm not sure if that is necessary. Making '--no-cone' required might stir up some noise that indicate how much of an impact the change would make. Thanks, -Stolee
