{"thread":{"id":"44781","subject":"[PATCH] am: add am.signoff add config variable","startedAt":"2016-12-28T17:41:42Z","lastAt":"2016-12-29T23:34:58Z","messageCount":11,"participants":["Eduardo Habkost","Stefan Beller","Eric Wong","Jacob Keller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"308454","messageId":"1482946838-28779-1-git-send-email-ehabkost@redhat.com","threadId":"44781","inReplyTo":null,"subject":"[PATCH] am: add am.signoff add config variable","fromName":"Eduardo Habkost","fromEmail":"ehabkost@redhat.com","sentAt":"2016-12-28T17:40:38Z","receivedAt":"2016-12-28T17:41:42Z","isPatch":true,"sender":{"key":"ehabkost@redhat.com","avatar":null},"body":"git-am has options to enable --message-id and --3way by default,\nbut no option to enable --signoff by default. Add a \"am.signoff\"\nconfig option.\n\nSigned-off-by: Eduardo Habkost \n---\n builtin/am.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/builtin/am.c b/builtin/am.c\nindex 31fb605..d2e0233 100644\n--- a/builtin/am.c\n+++ b/builtin/am.c\n@@ -154,6 +154,8 @@ static void am_state_init(struct am_state *state, const char *dir)\n \n \tgit_config_get_bool(\"am.messageid\", &state->message_id);\n \n+\tgit_config_get_bool(\"am.signoff\", &state->signoff);\n+\n \tstate->scissors = SCISSORS_UNSET;\n \n \targv_array_init(&state->git_apply_opts);\n-- \n2.7.4\n\n"},{"id":"308455","messageId":"CAGZ79kbRMYyaOmuqymx9dsLGdvX+iM9OMMQtQGS=uA+dO6_MVQ@mail.gmail.com","threadId":"44781","inReplyTo":"1482946838-28779-1-git-send-email-ehabkost@redhat.com","subject":"Re: [PATCH] am: add am.signoff add config variable","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-12-28T17:45:24Z","receivedAt":"2016-12-28T17:45:58Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Dec 28, 2016 at 9:40 AM, Eduardo Habkost wrote:\n> git-am has options to enable --message-id and --3way by default,\n> but no option to enable --signoff by default. Add a \"am.signoff\"\n> config option.\n\nI think this is a good idea (from a design standpoint and what the user needs).\n\nJust like e97a5e765d (git-am: add am.threeWay config variable), we'd\nprefer if you'd\nalso update Documentation/config.txt as well as a new test. :)\n\nThanks,\nStefan\n"},{"id":"308456","messageId":"20161228175047.GE3441@thinpad.lan.raisama.net","threadId":"44781","inReplyTo":"CAGZ79kbRMYyaOmuqymx9dsLGdvX+iM9OMMQtQGS=uA+dO6_MVQ@mail.gmail.com","subject":"Re: [PATCH] am: add am.signoff add config variable","fromName":"Eduardo Habkost","fromEmail":"ehabkost@redhat.com","sentAt":"2016-12-28T17:50:47Z","receivedAt":"2016-12-28T17:51:07Z","isPatch":true,"sender":{"key":"ehabkost@redhat.com","avatar":null},"body":"On Wed, Dec 28, 2016 at 09:45:24AM -0800, Stefan Beller wrote:\n> On Wed, Dec 28, 2016 at 9:40 AM, Eduardo Habkost wrote:\n> > git-am has options to enable --message-id and --3way by default,\n> > but no option to enable --signoff by default. Add a \"am.signoff\"\n> > config option.\n> \n> I think this is a good idea (from a design standpoint and what the user needs).\n> \n> Just like e97a5e765d (git-am: add am.threeWay config variable), we'd\n> prefer if you'd\n> also update Documentation/config.txt as well as a new test. :)\n\nSorry, I was using commit e97a5e765d as reference when adding the\nnew option, but I was looking at \"git log -p builtin/am.c\" and\ndidn't see the rest of commit. :)\n\nI will send a new version with the appropriate documentation and\ntest code.\n\n-- \nEduardo\n"},{"id":"308482","messageId":"20161229084701.GA3643@starla","threadId":"44781","inReplyTo":"1482946838-28779-1-git-send-email-ehabkost@redhat.com","subject":"Re: [PATCH] am: add am.signoff add config variable","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-12-29T08:47:01Z","receivedAt":"2016-12-29T08:55:58Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Eduardo Habkost wrote:\n> git-am has options to enable --message-id and --3way by default,\n> but no option to enable --signoff by default. Add a \"am.signoff\"\n> config option.\n\nI'm not sure this is a good idea. IANAL, but a sign-off\nhas some sort of legal meaning for this project (DCO)\nand that would be better decided on a patch-by-patch basis\nrather than a blanket statement.\n\nI don't add my SoB to patches (either my own or received) until\nI'm comfortable with it; and I'd rather err on the side of\nforgetting and being prodded to resubmit rather than putting\nan SoB on the wrong patch.\n\n\n(I'm barely online today, no rush needed in responding)\n"},{"id":"308494","messageId":"20161229154522.GB23595@thinpad.lan.raisama.net","threadId":"44781","inReplyTo":"20161229084701.GA3643@starla","subject":"Re: [PATCH] am: add am.signoff add config variable","fromName":"Eduardo Habkost","fromEmail":"ehabkost@redhat.com","sentAt":"2016-12-29T15:45:22Z","receivedAt":"2016-12-29T15:45:34Z","isPatch":true,"sender":{"key":"ehabkost@redhat.com","avatar":null},"body":"On Thu, Dec 29, 2016 at 08:47:01AM +0000, Eric Wong wrote:\n> Eduardo Habkost wrote:\n> > git-am has options to enable --message-id and --3way by default,\n> > but no option to enable --signoff by default. Add a \"am.signoff\"\n> > config option.\n> \n> I'm not sure this is a good idea. IANAL, but a sign-off\n> has some sort of legal meaning for this project (DCO)\n> and that would be better decided on a patch-by-patch basis\n> rather than a blanket statement.\n> \n> I don't add my SoB to patches (either my own or received) until\n> I'm comfortable with it; and I'd rather err on the side of\n> forgetting and being prodded to resubmit rather than putting\n> an SoB on the wrong patch.\n\nThis is an interesting point. I am not competent to argue about\nit, so I will let the Git developers decide.\n\n-- \nEduardo\n"},{"id":"308496","messageId":"A52AD4B5-8AB3-49E8-9EED-5ABBA91369D4@gmail.com","threadId":"44781","inReplyTo":"20161229084701.GA3643@starla","subject":"Re: [PATCH] am: add am.signoff add config variable","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-12-29T17:49:19Z","receivedAt":"2016-12-29T17:49:28Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On December 29, 2016 12:47:01 AM PST, Eric Wong wrote:\n>Eduardo Habkost wrote:\n>> git-am has options to enable --message-id and --3way by default,\n>> but no option to enable --signoff by default. Add a \"am.signoff\"\n>> config option.\n>\n>I'm not sure this is a good idea. IANAL, but a sign-off\n>has some sort of legal meaning for this project (DCO)\n>and that would be better decided on a patch-by-patch basis\n>rather than a blanket statement.\n>\n>I don't add my SoB to patches (either my own or received) until\n>I'm comfortable with it; and I'd rather err on the side of\n>forgetting and being prodded to resubmit rather than putting\n>an SoB on the wrong patch.\n>\n>\n>(I'm barely online today, no rush needed in responding)\n\nI don't know what is true for all projects, but I can't believe making it configurable would cause problems.... If you don't want it you don't have to enable it.\n\nI suppose your argument is that we shouldn't ever make it automatically but... I know that many people who receive email patches, apply them, then forward those to another group add their sign off as a mark of \"yes I certify that this is legally OK\" so I think this would be useful.\n\nThanks\nJake\n\n\n"},{"id":"308500","messageId":"xmqqtw9m5s5m.fsf@gitster.mtv.corp.google.com","threadId":"44781","inReplyTo":"20161229084701.GA3643@starla","subject":"Re: [PATCH] am: add am.signoff add config variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-29T21:42:13Z","receivedAt":"2016-12-29T21:42:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong writes:\n\n> Eduardo Habkost wrote:\n>> git-am has options to enable --message-id and --3way by default,\n>> but no option to enable --signoff by default. Add a \"am.signoff\"\n>> config option.\n>\n> I'm not sure this is a good idea. IANAL, but a sign-off\n> has some sort of legal meaning for this project (DCO)\n> and that would be better decided on a patch-by-patch basis\n> rather than a blanket statement.\n\nIANAL either, but we have been striving to keep output of\n\n $ git grep '\\.signoff' Documentation\n\nempty to keep Sign-off meaningful. \n\nAdding more publicized ways to add SoB without thinking will make it\nharder to argue against one who tells the court \"that log message\nends with a SoB by person X but it is very plausible that it was\ndone by inertia without person X really intending to certify what\nDCO says, and the SoB is meaningless\".\n\n> I don't add my SoB to patches (either my own or received) until\n> I'm comfortable with it; and I'd rather err on the side of\n> forgetting and being prodded to resubmit rather than putting\n> an SoB on the wrong patch.\n\n"},{"id":"308501","messageId":"20161229221605.GJ3441@thinpad.lan.raisama.net","threadId":"44781","inReplyTo":"xmqqtw9m5s5m.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] am: add am.signoff add config variable","fromName":"Eduardo Habkost","fromEmail":"ehabkost@redhat.com","sentAt":"2016-12-29T22:16:05Z","receivedAt":"2016-12-29T22:16:13Z","isPatch":true,"sender":{"key":"ehabkost@redhat.com","avatar":null},"body":"On Thu, Dec 29, 2016 at 01:42:13PM -0800, Junio C Hamano wrote:\n> Eric Wong writes:\n> \n> > Eduardo Habkost wrote:\n> >> git-am has options to enable --message-id and --3way by default,\n> >> but no option to enable --signoff by default. Add a \"am.signoff\"\n> >> config option.\n> >\n> > I'm not sure this is a good idea. IANAL, but a sign-off\n> > has some sort of legal meaning for this project (DCO)\n> > and that would be better decided on a patch-by-patch basis\n> > rather than a blanket statement.\n> \n> IANAL either, but we have been striving to keep output of\n> \n> $ git grep '\\.signoff' Documentation\n> \n> empty to keep Sign-off meaningful. \n> \n> Adding more publicized ways to add SoB without thinking will make it\n> harder to argue against one who tells the court \"that log message\n> ends with a SoB by person X but it is very plausible that it was\n> done by inertia without person X really intending to certify what\n> DCO says, and the SoB is meaningless\".\n\nThis sounds completely reasonable to me. I now see that the\nconfig option was already proposed in 2011 and the same arguments\nwere discussed. Sorry for the noise.\n\n> \n> > I don't add my SoB to patches (either my own or received) until\n> > I'm comfortable with it; and I'd rather err on the side of\n> > forgetting and being prodded to resubmit rather than putting\n> > an SoB on the wrong patch.\n> \n\n-- \nEduardo\n"},{"id":"308502","messageId":"CAGZ79kaAbCTsY_SddVMKMsLV0xyXNBFvxQ=J-20Cwdz31v4OwA@mail.gmail.com","threadId":"44781","inReplyTo":"xmqqtw9m5s5m.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] am: add am.signoff add config variable","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-12-29T22:18:48Z","receivedAt":"2016-12-29T22:18:54Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Dec 29, 2016 at 1:42 PM, Junio C Hamano wrote:\n> Eric Wong writes:\n>\n>> Eduardo Habkost wrote:\n>>> git-am has options to enable --message-id and --3way by default,\n>>> but no option to enable --signoff by default. Add a \"am.signoff\"\n>>> config option.\n>>\n>> I'm not sure this is a good idea. IANAL, but a sign-off\n>> has some sort of legal meaning for this project (DCO)\n>> and that would be better decided on a patch-by-patch basis\n>> rather than a blanket statement.\n>\n> IANAL either, but we have been striving to keep output of\n>\n> $ git grep '\\.signoff' Documentation\n\nTry again with -i ;)\nand you'll find format.signOff\n\n>\n> empty to keep Sign-off meaningful.\n>\n> Adding more publicized ways to add SoB without thinking will make it\n> harder to argue against one who tells the court \"that log message\n> ends with a SoB by person X but it is very plausible that it was\n> done by inertia without person X really intending to certify what\n> DCO says, and the SoB is meaningless\".\n\nI think we should be symmetrical, am is the opposite of\nformat-patch in the Git <-> email conversion.\n\nIf I were to follow your arguments here, we should revert\n1d1876e930 (Add configuration variable for sign-off to format-patch)\n\nOn the other hand, I would argue that thinking and typing things is\northogonal (the more you type, doesn't imply that you think harder\nor even at all).\n\n--\nHowever I think there is a use case for such an option\n(in the short term?) as e.g. you as the Git maintainer being employed\nhas the legal rights to sign off on pretty much any patch in Git.\nFor other projects this may be different.\n\nWhich is why a per-repository configurable thing is useful to\nsetup a default for your environment.\n\nIIUC long term we rather want to have easily configurable trailers\nfor am/format-patch/commit, such that you could configure to\nremove any \"ChangeId:\" footers before sending out, or adding\na \"Tested-by\" footer by a CI system or such?\n\n>\n>> I don't add my SoB to patches (either my own or received) until\n>> I'm comfortable with it;\n\ncomfortable is orthogonal to legal, specifically on the receiving side.\nI can understand using format-patch to send out unsigned patches\n(e.g. for heavy WIP things, \"please to not apply literally\")\n\n>> and I'd rather err on the side of\n>> forgetting and being prodded to resubmit rather than putting\n>> an SoB on the wrong patch.\n>\n"},{"id":"308503","messageId":"xmqqpoka5pb0.fsf@gitster.mtv.corp.google.com","threadId":"44781","inReplyTo":"CAGZ79kaAbCTsY_SddVMKMsLV0xyXNBFvxQ=J-20Cwdz31v4OwA@mail.gmail.com","subject":"Re: [PATCH] am: add am.signoff add config variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-29T22:43:47Z","receivedAt":"2016-12-29T22:43:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n>> IANAL either, but we have been striving to keep output of\n>>\n>> $ git grep '\\.signoff' Documentation\n>\n>>\n>> empty to keep Sign-off meaningful.\n>\n> Try again with -i ;)\n> and you'll find format.signOff\n\nMistakes happen. Finding an old mistake is not an excuse for you to\nmake the same one again. It is an opportunity to come up with a way\nto correct it without hurting existing users by designing a smooth\ntransition path.\n\n"},{"id":"308504","messageId":"CAGZ79kZFCDiYt6q52t5XMw0aB2qHA9ODVkCQJSxrGckSV3+O8A@mail.gmail.com","threadId":"44781","inReplyTo":"xmqqpoka5pb0.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] am: add am.signoff add config variable","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-12-29T23:34:51Z","receivedAt":"2016-12-29T23:34:58Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Dec 29, 2016 at 2:43 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>>> IANAL either, but we have been striving to keep output of\n>>>\n>>> $ git grep '\\.signoff' Documentation\n>>\n>>>\n>>> empty to keep Sign-off meaningful.\n>>\n>> Try again with -i ;)\n>> and you'll find format.signOff\n>\n> Mistakes happen. Finding an old mistake is not an excuse for you to\n> make the same one again.\n> It is an opportunity to come up with a way\n> to correct it without hurting existing users by designing a smooth\n> transition path.\n>\n\nknee jerk reaction:\n\n1) Document why format.signoff is bad (even after a long\n office discussion I am not fully convinced it is bad. That\n may be because I am biased as I find format.signoff *very*\n useful. The cumbersome contribution process as laid out\n by SubmittingPatches just got easier for me as I have one\n step less to worry about. I haven't made a mistake so far\n sending out crap where I'll throw a temper tantrum if you\n apply it.\n\n So I would expect a maintainer of a project that uses\n email based workflow to write that documentation giving\n reasons. That person is currently consuming the automatically\n signed off patches, so I'd want to know their line of thinking.\n\n2) If the config option is set, but no explicit sign off is given,\n put a different footer, e.g.\n git config format.signoff true &&\n git format-patch HEAD^\n may produce Auto-Signed-Off-By: ...\n whereas\n git -c format.signoff format-patch\n behaves the same as\n git format-patch --signoff\n that gives the Signed-Off-By as we know it.\n\n It is up to the upstream project to accept these new sign offs.\n\n3) (later) warn about the option if it is set, giving the text from 1)\n\n4) (a long time later) remove the option.\n\n--\nFor 2) I am not sure what we want there, because this\nhas to happen in collaboration with all the upstream projects that\nuse sign offs.\n\nWe could be subtle, i.e. just use all lowercase / all uppercase\nletters for this differentiation. Then automated tools that check for signoff\nare easily adjusted. e.g. The eclipse foundation disallows pushing for\nreview if any patch is missing a signoff; Gerrit can check for that.\nGerrit is not case sensitive when checking for a footer.\n\nI am not sure if there are any other tools out there that automatically\ncheck for that, but I would assume they are also case insensitive in\nsuch a case, as it is unclear to me how to properly capitalize the sign off.\n\nThis is an easy way forward for upstream projects., though confusing in\ncourt later on.\n--\nWe could also be non-subtle, very explicit, and each tool that\ncan add sign offs currently, needs to be explicit about itself:\n\n Configured-Formatpatch-Signed-Off: (for git format.signoff with config)\n # and others:\n Explicit-Formatpatch-Signed-Off:\n Git-Gui-Button-Clicked-Signed-Off:\n Git-Gui-Button-Shortcut-Signed-Off:\n\nOnce we have that we could add much more of these:\n Configured-Commit-Signed-Off:\n etc.\n\nYou can continue to sign off via just typing it, or by\n I-typed-it-signed-Off,\n--\nOne of the problems highlighted to me was that you could have accidentally\nconfigured format.signoff globally, but you're only allowed/desire to sign off\nin a particular repository, such that\n\n Repolocal-Configured-Formatpatch-Signed-Off:\n Global-Configured-Formatpatch-Signed-Off:\n\nmay be worth discussing.\n--\n"}]}