* [PATCH] am: add am.signoff add config variable @ 2016-12-28 17:40 Eduardo Habkost 2016-12-28 17:45 ` Stefan Beller 2016-12-29 8:47 ` Eric Wong 0 siblings, 2 replies; 11+ messages in thread From: Eduardo Habkost @ 2016-12-28 17:40 UTC (permalink / raw) To: git; +Cc: Paul Tan git-am has options to enable --message-id and --3way by default, but no option to enable --signoff by default. Add a "am.signoff" config option. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- builtin/am.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/builtin/am.c b/builtin/am.c index 31fb605..d2e0233 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -154,6 +154,8 @@ static void am_state_init(struct am_state *state, const char *dir) git_config_get_bool("am.messageid", &state->message_id); + git_config_get_bool("am.signoff", &state->signoff); + state->scissors = SCISSORS_UNSET; argv_array_init(&state->git_apply_opts); -- 2.7.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] am: add am.signoff add config variable 2016-12-28 17:40 [PATCH] am: add am.signoff add config variable Eduardo Habkost @ 2016-12-28 17:45 ` Stefan Beller 2016-12-28 17:50 ` Eduardo Habkost 2016-12-29 8:47 ` Eric Wong 1 sibling, 1 reply; 11+ messages in thread From: Stefan Beller @ 2016-12-28 17:45 UTC (permalink / raw) To: Eduardo Habkost; +Cc: git@vger.kernel.org, Paul Tan On Wed, Dec 28, 2016 at 9:40 AM, Eduardo Habkost <ehabkost@redhat.com> wrote: > git-am has options to enable --message-id and --3way by default, > but no option to enable --signoff by default. Add a "am.signoff" > config option. I think this is a good idea (from a design standpoint and what the user needs). Just like e97a5e765d (git-am: add am.threeWay config variable), we'd prefer if you'd also update Documentation/config.txt as well as a new test. :) Thanks, Stefan ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] am: add am.signoff add config variable 2016-12-28 17:45 ` Stefan Beller @ 2016-12-28 17:50 ` Eduardo Habkost 0 siblings, 0 replies; 11+ messages in thread From: Eduardo Habkost @ 2016-12-28 17:50 UTC (permalink / raw) To: Stefan Beller; +Cc: git@vger.kernel.org, Paul Tan On Wed, Dec 28, 2016 at 09:45:24AM -0800, Stefan Beller wrote: > On Wed, Dec 28, 2016 at 9:40 AM, Eduardo Habkost <ehabkost@redhat.com> wrote: > > git-am has options to enable --message-id and --3way by default, > > but no option to enable --signoff by default. Add a "am.signoff" > > config option. > > I think this is a good idea (from a design standpoint and what the user needs). > > Just like e97a5e765d (git-am: add am.threeWay config variable), we'd > prefer if you'd > also update Documentation/config.txt as well as a new test. :) Sorry, I was using commit e97a5e765d as reference when adding the new option, but I was looking at "git log -p builtin/am.c" and didn't see the rest of commit. :) I will send a new version with the appropriate documentation and test code. -- Eduardo ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] am: add am.signoff add config variable 2016-12-28 17:40 [PATCH] am: add am.signoff add config variable Eduardo Habkost 2016-12-28 17:45 ` Stefan Beller @ 2016-12-29 8:47 ` Eric Wong 2016-12-29 15:45 ` Eduardo Habkost ` (2 more replies) 1 sibling, 3 replies; 11+ messages in thread From: Eric Wong @ 2016-12-29 8:47 UTC (permalink / raw) To: Eduardo Habkost; +Cc: git, Paul Tan Eduardo Habkost <ehabkost@redhat.com> wrote: > git-am has options to enable --message-id and --3way by default, > but no option to enable --signoff by default. Add a "am.signoff" > config option. I'm not sure this is a good idea. IANAL, but a sign-off has some sort of legal meaning for this project (DCO) and that would be better decided on a patch-by-patch basis rather than a blanket statement. I don't add my SoB to patches (either my own or received) until I'm comfortable with it; and I'd rather err on the side of forgetting and being prodded to resubmit rather than putting an SoB on the wrong patch. (I'm barely online today, no rush needed in responding) ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] am: add am.signoff add config variable 2016-12-29 8:47 ` Eric Wong @ 2016-12-29 15:45 ` Eduardo Habkost 2016-12-29 17:49 ` Jacob Keller 2016-12-29 21:42 ` Junio C Hamano 2 siblings, 0 replies; 11+ messages in thread From: Eduardo Habkost @ 2016-12-29 15:45 UTC (permalink / raw) To: Eric Wong; +Cc: git, Paul Tan On Thu, Dec 29, 2016 at 08:47:01AM +0000, Eric Wong wrote: > Eduardo Habkost <ehabkost@redhat.com> wrote: > > git-am has options to enable --message-id and --3way by default, > > but no option to enable --signoff by default. Add a "am.signoff" > > config option. > > I'm not sure this is a good idea. IANAL, but a sign-off > has some sort of legal meaning for this project (DCO) > and that would be better decided on a patch-by-patch basis > rather than a blanket statement. > > I don't add my SoB to patches (either my own or received) until > I'm comfortable with it; and I'd rather err on the side of > forgetting and being prodded to resubmit rather than putting > an SoB on the wrong patch. This is an interesting point. I am not competent to argue about it, so I will let the Git developers decide. -- Eduardo ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] am: add am.signoff add config variable 2016-12-29 8:47 ` Eric Wong 2016-12-29 15:45 ` Eduardo Habkost @ 2016-12-29 17:49 ` Jacob Keller 2016-12-29 21:42 ` Junio C Hamano 2 siblings, 0 replies; 11+ messages in thread From: Jacob Keller @ 2016-12-29 17:49 UTC (permalink / raw) To: Eric Wong, Eduardo Habkost; +Cc: git, Paul Tan On December 29, 2016 12:47:01 AM PST, Eric Wong <e@80x24.org> wrote: >Eduardo Habkost <ehabkost@redhat.com> wrote: >> git-am has options to enable --message-id and --3way by default, >> but no option to enable --signoff by default. Add a "am.signoff" >> config option. > >I'm not sure this is a good idea. IANAL, but a sign-off >has some sort of legal meaning for this project (DCO) >and that would be better decided on a patch-by-patch basis >rather than a blanket statement. > >I don't add my SoB to patches (either my own or received) until >I'm comfortable with it; and I'd rather err on the side of >forgetting and being prodded to resubmit rather than putting >an SoB on the wrong patch. > > >(I'm barely online today, no rush needed in responding) I 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. I 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. Thanks Jake ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] am: add am.signoff add config variable 2016-12-29 8:47 ` Eric Wong 2016-12-29 15:45 ` Eduardo Habkost 2016-12-29 17:49 ` Jacob Keller @ 2016-12-29 21:42 ` Junio C Hamano 2016-12-29 22:16 ` Eduardo Habkost 2016-12-29 22:18 ` Stefan Beller 2 siblings, 2 replies; 11+ messages in thread From: Junio C Hamano @ 2016-12-29 21:42 UTC (permalink / raw) To: Eric Wong; +Cc: Eduardo Habkost, git, Paul Tan Eric Wong <e@80x24.org> writes: > Eduardo Habkost <ehabkost@redhat.com> wrote: >> git-am has options to enable --message-id and --3way by default, >> but no option to enable --signoff by default. Add a "am.signoff" >> config option. > > I'm not sure this is a good idea. IANAL, but a sign-off > has some sort of legal meaning for this project (DCO) > and that would be better decided on a patch-by-patch basis > rather than a blanket statement. IANAL either, but we have been striving to keep output of $ git grep '\.signoff' Documentation empty to keep Sign-off meaningful. Adding more publicized ways to add SoB without thinking will make it harder to argue against one who tells the court "that log message ends with a SoB by person X but it is very plausible that it was done by inertia without person X really intending to certify what DCO says, and the SoB is meaningless". > I don't add my SoB to patches (either my own or received) until > I'm comfortable with it; and I'd rather err on the side of > forgetting and being prodded to resubmit rather than putting > an SoB on the wrong patch. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] am: add am.signoff add config variable 2016-12-29 21:42 ` Junio C Hamano @ 2016-12-29 22:16 ` Eduardo Habkost 2016-12-29 22:18 ` Stefan Beller 1 sibling, 0 replies; 11+ messages in thread From: Eduardo Habkost @ 2016-12-29 22:16 UTC (permalink / raw) To: Junio C Hamano; +Cc: Eric Wong, git, Paul Tan On Thu, Dec 29, 2016 at 01:42:13PM -0800, Junio C Hamano wrote: > Eric Wong <e@80x24.org> writes: > > > Eduardo Habkost <ehabkost@redhat.com> wrote: > >> git-am has options to enable --message-id and --3way by default, > >> but no option to enable --signoff by default. Add a "am.signoff" > >> config option. > > > > I'm not sure this is a good idea. IANAL, but a sign-off > > has some sort of legal meaning for this project (DCO) > > and that would be better decided on a patch-by-patch basis > > rather than a blanket statement. > > IANAL either, but we have been striving to keep output of > > $ git grep '\.signoff' Documentation > > empty to keep Sign-off meaningful. > > Adding more publicized ways to add SoB without thinking will make it > harder to argue against one who tells the court "that log message > ends with a SoB by person X but it is very plausible that it was > done by inertia without person X really intending to certify what > DCO says, and the SoB is meaningless". This sounds completely reasonable to me. I now see that the config option was already proposed in 2011 and the same arguments were discussed. Sorry for the noise. > > > I don't add my SoB to patches (either my own or received) until > > I'm comfortable with it; and I'd rather err on the side of > > forgetting and being prodded to resubmit rather than putting > > an SoB on the wrong patch. > -- Eduardo ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] am: add am.signoff add config variable 2016-12-29 21:42 ` Junio C Hamano 2016-12-29 22:16 ` Eduardo Habkost @ 2016-12-29 22:18 ` Stefan Beller 2016-12-29 22:43 ` Junio C Hamano 1 sibling, 1 reply; 11+ messages in thread From: Stefan Beller @ 2016-12-29 22:18 UTC (permalink / raw) To: Junio C Hamano; +Cc: Eric Wong, Eduardo Habkost, git@vger.kernel.org, Paul Tan On Thu, Dec 29, 2016 at 1:42 PM, Junio C Hamano <gitster@pobox.com> wrote: > Eric Wong <e@80x24.org> writes: > >> Eduardo Habkost <ehabkost@redhat.com> wrote: >>> git-am has options to enable --message-id and --3way by default, >>> but no option to enable --signoff by default. Add a "am.signoff" >>> config option. >> >> I'm not sure this is a good idea. IANAL, but a sign-off >> has some sort of legal meaning for this project (DCO) >> and that would be better decided on a patch-by-patch basis >> rather than a blanket statement. > > IANAL either, but we have been striving to keep output of > > $ git grep '\.signoff' Documentation Try again with -i ;) and you'll find format.signOff > > empty to keep Sign-off meaningful. > > Adding more publicized ways to add SoB without thinking will make it > harder to argue against one who tells the court "that log message > ends with a SoB by person X but it is very plausible that it was > done by inertia without person X really intending to certify what > DCO says, and the SoB is meaningless". I think we should be symmetrical, am is the opposite of format-patch in the Git <-> email conversion. If I were to follow your arguments here, we should revert 1d1876e930 (Add configuration variable for sign-off to format-patch) On the other hand, I would argue that thinking and typing things is orthogonal (the more you type, doesn't imply that you think harder or even at all). -- However I think there is a use case for such an option (in the short term?) as e.g. you as the Git maintainer being employed has the legal rights to sign off on pretty much any patch in Git. For other projects this may be different. Which is why a per-repository configurable thing is useful to setup a default for your environment. IIUC long term we rather want to have easily configurable trailers for am/format-patch/commit, such that you could configure to remove any "ChangeId:" footers before sending out, or adding a "Tested-by" footer by a CI system or such? > >> I don't add my SoB to patches (either my own or received) until >> I'm comfortable with it; comfortable is orthogonal to legal, specifically on the receiving side. I can understand using format-patch to send out unsigned patches (e.g. for heavy WIP things, "please to not apply literally") >> and I'd rather err on the side of >> forgetting and being prodded to resubmit rather than putting >> an SoB on the wrong patch. > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] am: add am.signoff add config variable 2016-12-29 22:18 ` Stefan Beller @ 2016-12-29 22:43 ` Junio C Hamano 2016-12-29 23:34 ` Stefan Beller 0 siblings, 1 reply; 11+ messages in thread From: Junio C Hamano @ 2016-12-29 22:43 UTC (permalink / raw) To: Stefan Beller; +Cc: Eric Wong, Eduardo Habkost, git@vger.kernel.org, Paul Tan Stefan Beller <sbeller@google.com> writes: >> IANAL either, but we have been striving to keep output of >> >> $ git grep '\.signoff' Documentation > >> >> empty to keep Sign-off meaningful. > > Try again with -i ;) > and you'll find format.signOff Mistakes happen. Finding an old mistake is not an excuse for you to make the same one again. It is an opportunity to come up with a way to correct it without hurting existing users by designing a smooth transition path. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] am: add am.signoff add config variable 2016-12-29 22:43 ` Junio C Hamano @ 2016-12-29 23:34 ` Stefan Beller 0 siblings, 0 replies; 11+ messages in thread From: Stefan Beller @ 2016-12-29 23:34 UTC (permalink / raw) To: Junio C Hamano; +Cc: Eric Wong, Eduardo Habkost, git@vger.kernel.org, Paul Tan On Thu, Dec 29, 2016 at 2:43 PM, Junio C Hamano <gitster@pobox.com> wrote: > Stefan Beller <sbeller@google.com> writes: > >>> IANAL either, but we have been striving to keep output of >>> >>> $ git grep '\.signoff' Documentation >> >>> >>> empty to keep Sign-off meaningful. >> >> Try again with -i ;) >> and you'll find format.signOff > > Mistakes happen. Finding an old mistake is not an excuse for you to > make the same one again. > It is an opportunity to come up with a way > to correct it without hurting existing users by designing a smooth > transition path. > knee jerk reaction: 1) Document why format.signoff is bad (even after a long office discussion I am not fully convinced it is bad. That may be because I am biased as I find format.signoff *very* useful. The cumbersome contribution process as laid out by SubmittingPatches just got easier for me as I have one step less to worry about. I haven't made a mistake so far sending out crap where I'll throw a temper tantrum if you apply it. So I would expect a maintainer of a project that uses email based workflow to write that documentation giving reasons. That person is currently consuming the automatically signed off patches, so I'd want to know their line of thinking. 2) If the config option is set, but no explicit sign off is given, put a different footer, e.g. git config format.signoff true && git format-patch HEAD^ may produce Auto-Signed-Off-By: ... whereas git -c format.signoff format-patch behaves the same as git format-patch --signoff that gives the Signed-Off-By as we know it. It is up to the upstream project to accept these new sign offs. 3) (later) warn about the option if it is set, giving the text from 1) 4) (a long time later) remove the option. -- For 2) I am not sure what we want there, because this has to happen in collaboration with all the upstream projects that use sign offs. We could be subtle, i.e. just use all lowercase / all uppercase letters for this differentiation. Then automated tools that check for signoff are easily adjusted. e.g. The eclipse foundation disallows pushing for review if any patch is missing a signoff; Gerrit can check for that. Gerrit is not case sensitive when checking for a footer. I am not sure if there are any other tools out there that automatically check for that, but I would assume they are also case insensitive in such a case, as it is unclear to me how to properly capitalize the sign off. This is an easy way forward for upstream projects., though confusing in court later on. -- We could also be non-subtle, very explicit, and each tool that can add sign offs currently, needs to be explicit about itself: Configured-Formatpatch-Signed-Off: (for git format.signoff with config) # and others: Explicit-Formatpatch-Signed-Off: Git-Gui-Button-Clicked-Signed-Off: Git-Gui-Button-Shortcut-Signed-Off: Once we have that we could add much more of these: Configured-Commit-Signed-Off: etc. You can continue to sign off via just typing it, or by I-typed-it-signed-Off, -- One of the problems highlighted to me was that you could have accidentally configured format.signoff globally, but you're only allowed/desire to sign off in a particular repository, such that Repolocal-Configured-Formatpatch-Signed-Off: Global-Configured-Formatpatch-Signed-Off: may be worth discussing. -- ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-12-29 23:34 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-12-28 17:40 [PATCH] am: add am.signoff add config variable Eduardo Habkost 2016-12-28 17:45 ` Stefan Beller 2016-12-28 17:50 ` Eduardo Habkost 2016-12-29 8:47 ` Eric Wong 2016-12-29 15:45 ` Eduardo Habkost 2016-12-29 17:49 ` Jacob Keller 2016-12-29 21:42 ` Junio C Hamano 2016-12-29 22:16 ` Eduardo Habkost 2016-12-29 22:18 ` Stefan Beller 2016-12-29 22:43 ` Junio C Hamano 2016-12-29 23:34 ` Stefan Beller
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).