Re: [PATCH v5] submodule merge: update conflict error message

From: Calvin Wan <calvinwan@google.com>
Date: Jul 19, 2022, 19:30 UTC
Message-ID: <CAFySSZDqMVV=Zv9hVa+S9=fhuvCTZvZUL50qMfSJHh4KTpm5RA@mail.gmail.com>
In-Reply-To: <xmqqzgh5d2s5.fsf@gitster.g>

> The stuff to help reviewers who may have seen earlier round are
> usually written in the cover letter, or after the three-dash line.

Ah I see since this patch doesn't have a cover letter, I should put all the reviewer-centric stuff after the three-dash line.

Show 8 quoted lines

> > +     if (csub && csub->nr > 0) {
> > +             int i;
> > +             printf(_("Recursive merging with submodules currently only supports trivial cases.\n"
> > +                     "Please manually handle the merging of each conflicted submodule.\n"
> > +                     "This can be accomplished with the following steps:\n"));
>
> This makes me wonder if these "helpful but verbose" messages should
> use the advise mechanism.

I agree. The only loss of information if someone turned off this message would be the commit id that possibly needs to be merged.

> Also, those reviewers who care about l10n
> may suggest use of printf_ln() to lose the LF at the end of these
> messages (i.e. not just the above one, but others we see below as
> well).

ack