Re: [PATCH v7 6/7] submodule: refactor show_submodule_summary with helper function

From: Jacob Keller <jacob.keller@gmail.com>
Date: Aug 18, 2016, 21:08 UTC
Message-ID: <CA+P7+xpQBdr1rzOPazLFhOW8cdk2ePCiqsykGqCmStvt--UMaQ@mail.gmail.com>
In-Reply-To: <xmqqbn0p6bvu.fsf@gitster.mtv.corp.google.com>

On Thu, Aug 18, 2016 at 1:49 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 22 quoted lines

> Jacob Keller <jacob.keller@gmail.com> writes:
>
>>>>> If we were to change those, we could discuss if we want to go with
>>>>> full sentences
>>>>> all the time:
>>>>>
>>>>>     submodule is new
>>>>>     submodule is deleted
>>>>>     submodule is not initialized
>>>>
>>>> I agree, I'll make a new patch that does this as a cleanup prior to
>>>> this re-work.
>>> ...
>> Sorry for being unclear myself, too. I'm keeping it as "not
>> initialized" and updating the description of the patch that changed it
>> from "not checked out" to "not initialized"
>
> Whether it is done inside or outside the scope of this series, the
> other two "is new"/"is deleted" updates look very sensible ones to
> make in the longer run.  I'd further suggest to unify "commits not
> present" and "revision walker failed" into one error class.  From
> the end user's point of view, they aren't very different.

That won't work exactly due to the new way we separate the header format from the revision walk. I can change the error case though. The new show_submodule_header will not actually attempt a revision walk at all and thus won't know to display "revision walker failed" at the same place as the header fails. However I think almost all of the time it should succeed if commits are present.

Thanks, Jake