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

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Mar 23, 2017, 00:53 UTC
Message-ID: <20170323005341.GH26108@aiede.mtv.corp.google.com>
In-Reply-To: <20170323004329.15892-2-sbeller@google.com>

Stefan Beller wrote:

Show 7 quoted lines

> 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 think this would be easier to understand if it were two patches: one that switched to --porcelain=2 with no change in behavior, and another that took advantage of --porcelain=2 to return richer information. As is, I had trouble verifying that this isn't going to break anything --- there's not enough local information here and in submodule.h to tell what callers may rely on and I didn't audit callers.

Thanks and hope that helps, Jonathan