Thanks for your review. >> >> The previous change add "--merge-base" option in order to allow users >> to > > s/add/added/ ? Done. > 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 > Done. > 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. > > + 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.
