Re: [PATCH 1/3] submodule.c: port is_submodule_modified to use porcelain 2

[Stefan Beller <sbeller@google.com>]

On Wed, Mar 22, 2017 at 11:09 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> Stefan Beller wrote:
>>
>>> Migrate 'is_submodule_modified' to the new porcelain format of
>>> git-status.
>>>
>>> As the old porcelain only reported ' M' for submodules, no
>>> matter what happened inside the submodule (untracked files,
>>> changes to tracked files or move of HEAD), the new API
>>> properly reports the different scenarios.
>> [...]
>>>  submodule.c | 53 ++++++++++++++++++++++++-----------------------------
>>>  1 file changed, 24 insertions(+), 29 deletions(-)
>>
>> Neat.  Is this something that could be covered in tests, or should I
>> be patient and rely on patch 3/3 for that?

I am not sure how to cover it in tests properly, as we do not expose this
function to the outside directly.

This function is used in only one place (diff-lib.c, in
match_stat_with_submodule,
which itself is used in run_diff_files and get_stat_data), which is deep down
in the diff library.

>> I think this would be easier to understand if it were two patches: one
>> that switched to --porcelain=2 with no change in behavior,

I don't think so as it would double the code to review.
I'll see if I can present this conversion in an easier way.

> That sounds like a sensible organization.

ok.

Thanks,
Stefan