Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes: > 'git-format-patch' [-k] [-o <dir> | --stdout] [--thread] > [--attach[=<boundary>] | --inline[=<boundary>]] > + [--reviewed-by=<ident>] > [-s | --signoff] [<common diff options>] > [-n | --numbered | -N | --no-numbered] > [--start-number <n>] [--numbered-files] What's the expected workflow this patch intends to help? - You see a patch by somebody, you look at it deeply, you apply to your tree (presumably with your own Signed-off-by). - You inspect the result further, and decide it is good. - You format-patch with the option, which would now have a Reviewed-by: too. - You send it out. If so, it might make sense to simply always use the committer ident. If the person who adds the reviewed-by is trusted so much that her reviewed-by counts, the commits might even be transfered with "Please pull". In such a case, the workflow might become: - You see a patch by somebody, you look at it deeply, you apply to your tree (presumably with your own Signed-off-by). - You inspect the result further, and decide it is good. - You run "rebase --add-reviewed-by" to prepare a series on a branch to be pulled from. - You send a request-pull. In that workflow, it would also make sense to use the committer ident. I am trying to come up with a plausible workflow that wants to add somebody else's reviewed-by. - You send out your patch to the list. People give comments, you reroll, you get more comments, eventually people say "Ah, that's good, Ack." and/or "I am not the primary person who knows this area, but I reviewed it and I know my reviewed-by would count, so here is my Ok". - You format-patch the final version, with Acked-by and Reviewed-by adding other people's names. Then I think it makes sense to take names of other people if that is the case. You probably meant that that is the expected workflow, as you can give more than one of these options. But people who read the documentation should not have to guess. > diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh > index b2b7a8d..e2ff94f 100755 > --- a/t/t4014-format-patch.sh > +++ b/t/t4014-format-patch.sh > @@ -230,4 +230,30 @@ test_expect_success 'shortlog of cover-letter wraps overly-long onelines' ' > > ' > > +cat > expect << EOF > + > +Reviewed-by: Ken Robinson > + > +Reviewed-by: Sergey Rachmaninov > + > +Reviewed-by: Ronny O Sullivan > +Reviewed-by: Mickey Mouse > +Reviewed-by: Mahatma Gandhi > +EOF > + > +test_expect_success '--reviewed-by' ' > + > + echo reviewed > foo && > + test_tick && > + git commit -m "Reviewed" -m "Reviewed-by: Ken Robinson" \ > + -m "Reviewed-by: Sergey Rachmaninov" \ > + -m "Reviewed-by: Ronny O Sullivan" foo && > + git format-patch --reviewed-by="Mickey Mouse" \ > + --reviewed-by="Sergey Rachmaninov" \ > + --reviewed-by="Mahatma Gandhi" -1 HEAD && > + sed -e "1,/^Cc: /d" -e "/^---/,\$d" < 0001-Reviewed.patch > output && > + git diff expect output > + > +' Why not use a single -m for the first three reviewed-bys, instead of making them into separate paragraphs using multiple -m? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
