Re: [PATCH v3 2/2] worktree add: add --orphan flag

> On Fri, Nov 18, 2022 at 8:44 PM Jacob Abel <jacobabel@nullpo.dev> wrote:
> > On 22/11/15 10:08PM, Ævar Arnfjörð Bjarmason wrote:
> > > > +   test_must_fail git worktree add --orphan existingbranch orphandir2 &&
> > > > +   test ! -d orphandir2
> > >
> > > I'm not sure about "worktree" behavior, but maybe this "test ! -d" wants
> > > to be a "test_path_is_missing"?
> > >
> > > In general we try to test what a thing is, not what it isn't, in this
> > > case don't we fail to create the dir entirely? So "not exists" would
> > > cover it?
> >
> > Ah yes that would be preferable. I've updated it for v4.
> >
> > This shows up in the file in a few other places in this file as well
> > (from before this patch). Should I make the changes there as well and put
> > those changes into an additional patch in this patchset?
>
> With my reviewer's hat on, my goal is to help the series land in
> Junio's tree, which means I'd like to see fewer changes with each
> iteration. Adding a new patch which is only tangentially related to
> what the series wants to achieve isn't a priority, and could end up
> delaying acceptance of the series if problems in the new patch end up
> requiring additional rerolls.
>
> So, yes, you could do that cleanup as a preparatory patch in the
> series if you want to tackle it. It would be an appropriate cleanup
> since you're working on code nearby. Or it could be done as a follow
> up to this series. Given how small the cleanup patch would likely be,
> it may not make a difference one way or the other, especially if the
> commit message explains the change well (for instance, by paraphrasing
> what Ævar said about "testing what a thing is, not what it isn't").