Re: [PATCH v3 2/4] format-patch: add '--base' option to record base tree info

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 1, 2016, 16:00 UTC
Message-ID: <xmqqpou9jp4b.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160401133801.GA2915@yexl-desktop>

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

Show 7 quoted lines

> On Thu, Mar 31, 2016 at 10:38:04AM -0700, Junio C Hamano wrote:
>
>>The contents of this look OK, but does it format correctly via
>>AsciiDoc?  I suspect that only the first paragraph up to "of this
>>shape:" would appear correctly and all the rest would become funny.
>
> Sorry, just heard of AsciiDoc, I will try to use it to do the right format work.

Please make sure "make -C Documentation" produces sensible output for *.1 (manpage) and *.html.

Show 11 quoted lines

>>> +	init_revisions(&revs, NULL);
>>> +	revs.max_parents = 1;
>>> +	base->object.flags |= UNINTERESTING;
>>> +	add_pending_object(&revs, &base->object, "base");
>>> +	for (i = 0; i < total; i++) {
>>> +		list[i]->object.flags |= 0;
>>
>>What does this statement do, exactly?  Are you clearing some bits
>>but not others, and if so which ones?
>
> My mistake, it's useless and should be removed.

It probably make sense to do "&= ~UNINTERESTING" there, though. You are adding one UNINTERESTING object (i.e. the base) and adding objects that are on the list[] as interesting.

Show 6 quoted lines

>>This shows the patches in the order discovered by the revision
>>traversal, which typically is newer to older.  Is that intended?
>>Is it assumed that the order of the patches does not matter?
>
> The prerequisite patches should show in topological order, thus robot
> could parse them one by one and apply the patches in reverse order.

If you have history where base is B, with three prerequisites 1-2-3, before the patch series A-B-C, i.e.

	B---1---2---3---A---B---C

if you are showing "base-commit: B" as the first line in the base tree information block, it would be natural to expect that the prerequisite patch ids are listed for 1 and then 2 and then finally 3, i.e.

	base-commit: B
        prerequisite-patch-id: 1
        prerequisite-patch-id: 2
        prerequisite-patch-id: 3

no?

Also I know _you_ intend to consume this by robot, but it makes me wonder if with a minimum update you can make the output also more useful for bystander humans. A mailing list participant may

 - see an early round of a series that interests her,
 - try to apply them to her tree,
 - find that the series does not apply, but
 - sees that a block to help identify to what tree the series is
   meant to apply.

With a list of 40-hex alone, she may not be able to figure out the prerequisites, but if there is some other clue that helps her to identify the base commit and these patches, she may be able to construct a tree that is close enough. Maybe you can help her by appending the title of the commit and patches at the end of these lines?

This is not a strong suggestion (yet); I am thinking aloud at this point, without knowing how much it would help in practice to do so.