Re: [PATCH v2 1/1] t3600: use test_path_is_dir and test_path_is_file

>
> We prefer to use imperative mode when talking about what a patch does,
> as if the author were to give orders to the code base.  So e.g.
> instead of
>
>   This patch will ...
>
> we would usually write something like this:
>
>   Replace 'test -(d|f)' calls in t3600 with the corresponding helper
>   functions.
>

>
> >  test_expect_success 'Recursive with -r -f' '
> >       git rm -f -r frotz &&
> > -     ! test -f frotz/nitfol &&
> > -     ! test -d frotz
> > +     ! test_path_is_file frotz/nitfol &&
> > +     ! test_path_is_dir frotz
> >  '
>
> These should rather use the test_path_is_missing helper function.
>
> However, if the directory 'frotz' is missing, then surely
> 'frotz/nitfol' could not possibly exist either, could it?  I'm not
> sure why this test (and a couple of others) checks both, and wonder
> whether the redundant check for the file inside the supposedly
> non-existing directory could be removed.
>

> Furthermore, there are a couple of place where the '!' is not in front
> of the whole 'test' command but is given as an argument, e.g.:
>
>   test ! -f file
>
> Please convert those cases as well.
>