On Mon, Jan 10, 2022 at 10:01 AM Victoria Dye <vdye@xxxxxxxxxx> wrote: > > Elijah Newren wrote: > > On Mon, Jan 10, 2022 at 7:47 AM Victoria Dye <vdye@xxxxxxxxxx> wrote: > >> > >> Elijah Newren wrote: > >>> On Tue, Jan 4, 2022 at 9:37 AM Victoria Dye via GitGitGadget > >>> <gitgitgadget@xxxxxxxxx> wrote: > >>>> > >>>> From: Victoria Dye <vdye@xxxxxxxxxx> > >>>> ... > > Sorry about that - when I wrote the first version of this series in the > `microsoft/git` fork, `git add` hadn't been updated to reject out-of-cone > untracked files as it is in [1]. It's my mistake for not double-checking > that it was still the case, apologies for wasting your time on re-explaining > this. No worries; there's lots of moving parts in the sparse world. > In any case, I'll update the test comment and commit message per your > suggestion: > > >>> I might buy that `git update-index` is lower level and should be > >>> considered the same as `git add --sparse`, but the comment should > >>> mention that and try to sell why update-index should be equivalent to > >>> that instead of to `git add`. > > I'm leaning only slightly towards the current behavior (and will update the > comment accordingly), but I'm happy to change it if the reasoning isn't as > strong as that of another approach. > > [1] 105e8b014b (add: fail when adding an untracked sparse file, 2021-09-24) I'm also leaning slightly towards keeping the current behavior and just updating the comment. ... > >> I understand why you find it buggy, but I am not making baseless assumptions > >> about the correctness (or lack thereof) of the current behavior. > > > > To be clear, the fact that the behavior was there for a decade would > > typically be basis enough for an assumption (in my opinion), and I > > wouldn't have faulted folks for making it. I might well have done so > > myself. My reasoning was just that I was getting confused by the > > negations and trying to understand the testcase, and when I started to > > unravel it, I found what looked like a possible inconsistency. > > > > Anyway, it's clear here you've actually dug a lot deeper and know the > > history here. In contrast, I was making assumptions about the history > > (and ones that weren't correct, though I'd argue my assumptions > > weren't baseless)... > > > > Your assumptions were completely valid, I apologize if my response came off > as implying otherwise. I'll try to use the comments on the tests to clarify > their behavior as much as possible, hopefully reducing some confusion around > all the multiple-negative flags & options. Oh, no, not at all. I was just worried that my earlier response might have come across poorly and could be read as criticizing assumptions I (incorrectly) _thought_ you were making. I wasn't sure if that was actually implied in your wording, but just to be careful I was just trying to specify that I think the level of effort of your submissions is totally appropriate -- even if you had been making the assumptions I thought you had been. And I was pointing out that I, after all, had made and will continue to make assumptions too. The point of review is getting a second pair of eyes, because they can help catch issues; and finding those issues often includes double-checking unspoken assumptions. Anyway, I think we're on the same page. Thanks for your hard work here; looking forward to seeing your reroll!
