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

On Fri, Apr 01, 2016 at 09:00:20AM -0700, Junio C Hamano wrote:

Ye Xiaolong [off-list ref] writes:

quoted

On Thu, Mar 31, 2016 at 10:38:04AM -0700, Junio C Hamano wrote:

quoted

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.

OK.

quoted

quoted

quoted

+	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.

Yeah, it does make sense, I'll do the change.

quoted

quoted

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?

I think this sounds more sensible than what I had thought, I'll adjust 
the showing sequence accordingly.

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.

Thanks for the suggestions, actually it is what we have planned for next
step to make the output info more friendly for human being, I'll
follow up on it.

Thanks,
Xiaolong.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html