Re: [PATCH v4] diff: do not show submodule with untracked files as "-dirty"

>
> What happened to the first/introductory paragraph that was
> excellently written in the previous round?
>
> The usaul way to compose a log message is to
>
>  - Give an observation on how the current system work in the present
>    tense (so no need to say "Currently X is Y", just "X is Y"), and
>    discuss what you perceive as a problem in it.
>
>  - Propose a solution (optional---often, problem description
>    trivially leads to an obvious solution in reader's minds).
>
>  - Give commands to the codebase to "become like so".
>
> The first paragraph you had in the previous round was a good example
> of the "observation".  What we see above is only the "commands"
> part.  To those who have been intimately following the discussion,
> it often is understandable without both, but we are not writing for
> those who review the patches.  We are writing for future readers who
> are not aware of these exchanges we are having, so we should give
> something to prepare them by setting the stage and stating the
> objective.

>
> > diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
> > index ca04fac417..98e46ad1ae 100755
> > --- a/t/t3701-add-interactive.sh
> > +++ b/t/t3701-add-interactive.sh
> > @@ -761,7 +761,7 @@ test_expect_success 'setup different kinds of dirty submodules' '
> >               echo dirty >>initial &&
> >               : >untracked
> >       ) &&
> > -     git -C for-submodules diff-files --name-only >actual &&
> > +     git -C for-submodules diff-files --name-only --ignore-submodules=none >actual &&
> >       cat >expected <<-\EOF &&
> >       dirty-both-ways
> >       dirty-head
>
> Hmph, it seems that you chose to give options that are no longer the
> default to command invocations, so that the expected output from the
> previous world order before this patch can be reused, but is that a
> sensible way to go?  If we are changing the default behaviour, shouldn't
> we rather be testing that new behaviour, I wonder?
>

>
> By the way, in builtin/describe.c there is an invocation of
> "diff-index" without any --ignore-submodules=<what> option.
>
>         /* diff-index command arguments to check if working tree is dirty. */
>         static const char *diff_index_args[] = {
>                 "diff-index", "--quiet", "HEAD", "--", NULL
>         };
>
> Would the behaviour of diff-index invocation in "git describe --dirty"
> affected by the change of the default in any way?