Re: [PATCH v2 3/4] format-patch: introduce --base=auto option

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 24, 2016, 17:01 UTC
Message-ID: <xmqq37rfajy1.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160324041925.GB26582@yexl-desktop>

Ye Xiaolong <xiaolong.ye@intel.com> writes:

Show 11 quoted lines

> On Wed, Mar 23, 2016 at 11:25:41AM -0700, Junio C Hamano wrote:
>>I also do not see the point of showing "parent id" which as far as I
>>can see is just a random commit object name and show different
>>output that is not even described what it is.  It would be better to
>
> Here is our consideration:
> There is high chance that branch_get_upstream will retrun NULL(thus we
> are not able to get exact base commit), since developers may checkout
> branch from a local branch or a commit and haven't set "--set-upstream-to"
> to track a remote branch, in this case, we want to provide likely useful
> info(here is parent commit id and patch id)

I do not agree with that reasoning at all, primarily because your "likely useful" is not justfied.

Could you explain what makes you think that it is "likely" that the commit that matches "parent commit id" is available to the recipient of the patch?

Whatever the reason is, if it _is_ likely, then I do not see a point in spending cycle to do get-upstream and merge-base, or giving an option to the user to explicitly specify the base. Given that this series does these things, I'd have to conclude your "likely useful" is "might possibly turn out to be useful in some cases if you are lucky but is no way reliable" at best.

Rather than throwing an unreliable information in the output and blindly proceeding, I'd think you would want to error out and tell the user to explicitly give the base without producing the patch output. That way, you will not get patches with unreliable information.

Suggesting to use set-upstream-to when you give that error message may also be helpful.