On Thu, Nov 15, 2012 at 6:03 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Brandon Casey <drafnel@xxxxxxxxx> writes: > >> Detect whether the s-o-b already exists in the commit footer and refrain >> from adding a duplicate. > > If you are trying to forbid > > git cherry-pick -s $other > > from adding s-o-b: A when $other ends with these two existing s-o-b: > > s-o-b: A > s-o-b: B > > then I think that is a wrong thing to do. > > In such a case, the resulting commit should gain another s-o-b from > A to record the provenance as a chain of events. A originally wrote > the patch, B forwarded it (possibly with his own tweaks), and then A > picked it up and recorded the result to the history, possibly with a > final tweak or two. Hmm. I've never thought that it was necessary to add an additional sob for the patches that I've cherry picked that I had previously signed-off-on. I considered one sign-off to be enough. In your example, A is the committer and the patch set already contains A's sign-off. For me that indicates that A still considers the commit to comply with whatever s-o-b implies. We also seem to have a few tools to help people avoid adding duplicate sob's, like the current behavior of format-patch and the sample commit-msg hook. I did some quick searching through the kernel commits to try to find some examples that could set a precedence. I didn't find anything that supported either argument. I didn't see any commits that were cherry-picked _and_ had an existing sob that was not the last sob. I didn't see any that had duplicate sob lines either. I'm not mentioning this to say that lack of a prior use should mean we should actively disallow the practice of adding duplicate sob's, I'm just providing it as a data point. I've always thought that the reason that 'commit -s' and 'cherry-pick -s' checked only the last line of the commit message was simply that a full scan of the footer had not been implemented. Whichever behavior is determined to be the right way for git to do it, format-patch should be brought in-line with the others and be built on top of the code in sequencer.c. So, if git _should_ create duplicate sob's then this patch should just be dropped. Duy's unification patch can just be built on top of sequencer.c:append_signoff() without bringing over any of the duplicate sob detection from log-tree.c. -Brandon -- 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