RE: [Internet]Re: [PATCH v4 2/2] merge-tree.c: support --merge-base in conjunction with --stdin
Show 5 quoted lines
>>
>> The previous change add "--merge-base" option in order to allow users
>> to
>
> s/add/added/ ?
Show 5 quoted lines
> Perhaps something like:
>
> """
> The previous commit added a `--merge-base` option in order to allow using a specified merge-base for the merge. Extend the input accepted by `--stdin` to also allow a specified merge-base with each merge requested. For example:
>
> printf "--merge-base=<b3> <b1> <b2>" | git merge-tree --stdin
>
Show 5 quoted lines
> However, I'm a bit curious about using `--merge-base=` on the input line as opposed to just using a simpler marker; something like
>
> printf "<b3> -- <b1> <b2>" | git merge-tree --stdin
>
> (which follows a precedent set by git-merge-recursive). Your version is a bit more self-documenting, but what if we want to allow users to specify multiple merge bases in the future (for use in passing to merge_incore_recursive())? Is it annoying to need to prefix each one?
Actually, I thought about it at the time, but didn't think of any good input format. Your idea is great, I think I'll use it.
> I'm not sure "passed into the input line" will be clear to readers.
> Perhaps we want to have --stdin documented (looks like I forgot to do that in my series; oops), mentioning the input format.
I just added the documentation for --input, please see what else needs to be improved.
Show 8 quoted lines
> > + printf "1\0" >>expect &&
> > + git merge-tree --write-tree -z --merge-base=c2 c1 c3 >>expect &&
> > + printf "\0" >>expect &&
> > + test_cmp expect actual
> > +'
>
> This last test seems odd. You are merely testing that the output of "git merge-tree --stdin" matches the output of repeated calls to "git merge-tree", not that the merges involved produce correct results?
> Maybe that's fine since earlier tests verify that individual merge-tree calls are doing the right thing, I was just a bit surprised. Maybe a comment in the code that this was your intent would be helpful?
Yeah, I have added the comment.
kylezhao (赵柯宇) <kylezhao@tencent.com>